Андрей Карпов

21 Май 2014

Теги:

#Cpp

Tesseract. Распознаем ошибки в системе распознавания

21 Май 2014

Автор: Андрей Карпов

Tesseract
Результаты проверки
Заключение

Tesseract - свободная компьютерная программа для распознавания текстов, разрабатываемая компанией Google. В описании проекта говорится: "Tesseract is probably the most accurate open source OCR engine available". А давайте попробуем, сможет ли статический анализатор PVS-Studio распознать какие-то ошибки в этом проекте.

Tesseract

Tesseract - свободная компьютерная программа для распознавания текстов, разрабатывавшаяся Hewlett-Packard с середины 1980-х по середину 1990-х, а затем 10 лет "пролежавшая на полке". В августе 2006 г. Google купил её и открыл исходные тексты под лицензией Apache 2.0 для продолжения разработки. В настоящий момент программа уже работает с UTF-8, поддержка языков (включая русский) осуществляется с помощью дополнительных модулей. [описание взято из Wikipedia]

Исходный код проекта доступен на сайте Google Code: https://code.google.com/p/tesseract-ocr/

Объем исходного кода около 16 мегабайт.

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

Приведу фрагменты кода, на которые я обратил внимание, просматривая отчёт PVS-Studio. Возможно я что-то упустил. Поэтому создателям Tesseract целесообразно провести проверку самостоятельно. Пробная версия, активна 7 дней, что более чем достаточно для такого небольшого проекта. Ну а потом им решать, хотят они регулярно использовать инструмент и находить опечатки, или нет.

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

Неудачное деление

void LanguageModel::FillConsistencyInfo(....)
{
  ....
  float gap_ratio = expected_gap / actual_gap;
  if (gap_ratio < 1/2 || gap_ratio > 2) {
    consistency_info->num_inconsistent_spaces++;
  ....
}

Предупреждения PVS-Studio: V636 The '1 / 2' expression was implicitly casted from 'int' type to 'float' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. language_model.cpp 1163

Переменную 'gap_ratio' хотят сравнить со значением 0.5. К сожалению, выбран неудачный способ записать 0.5. Деление 1/2 является целочисленным и даёт в результате 0.

Корректный код должен быть таким:

if (gap_ratio < 1.0f/2 || gap_ratio > 2) {

Или таким:

if (gap_ratio < 0.5f || gap_ratio > 2) {

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

Фрагменты кода, которые стоит проверить:

baselinedetect.cpp 110
bmp_8.cpp 983
cjkpitch.cpp 553
cjkpitch.cpp 564
mfoutline.cpp 392
mfoutline.cpp 393
normalis.cpp 454

Опечатка в сравнении

uintmax_t streamtoumax(FILE* s, int base) {
  int d, c = 0;
  ....
  c = fgetc(s);
  if (c == 'x' && c == 'X') c = fgetc(s);
  ....
}

Предупреждение PVS-Studio: V547 Expression 'c == 'x' && c == 'X'' is always false. Probably the '||' operator should be used here. scanutils.cpp 135

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

if (c == 'x' || c == 'X') c = fgetc(s);

Неопределённое поведение

Обнаружилась одна интересная конструкция. Я такого ещё не видел:

void TabVector::Evaluate(....) {
  ....
  int num_deleted_boxes = 0;
  ....
  ++num_deleted_boxes = true;
  ....
}

Предупреждение PVS-Studio: V567 Undefined behavior. The 'num_deleted_boxes' variable is modified while being used twice between sequence points. tabvector.cpp 735

Не понятно, что хотел сказать этим кодом автор. Скорее всего этот код является последствием опечатки.

Результат выражения предсказать невозможно. Переменная 'num_deleted_boxes' может быть увеличена как до присваивания, так и после. Причина - переменная меняется дважды в одной точке следования.

Остальные ошибки, приводящие к неопределенному поведению, связаны с использованием сдвигов. Рассмотрим пример:

void Dawg::init(....)
{
  ....
  letter_mask_ = ~(~0 << flag_start_bit_);
  ....
}

Предупреждение V610 Undefined behavior. Check the shift operator '<<. The left operand '~0' is negative. dawg.cpp 187

Выражение '~0' имеет тип 'int' и равно значению '-1'. Сдвиг отрицательных значений приводит к неопределённому поведении. То, что программа может корректно работать, является везением и не больше того. Можно исправить недочёт, сделав '0' беззнаковым:

letter_mask_ = ~(~0u << flag_start_bit_);

Однако это ещё не всё. Анализатор выдаёт на эту строку ещё одно предупреждение:

V629 Consider inspecting the '~0 << flag_start_bit_' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. dawg.cpp 187

Дело в том, что переменная 'letter_mask_' имеет тип 'uinT64'. Как я понимаю, может быть необходимо записывать единицы в старшие 32 бита. В таком случае, созданное выражение некорректно. Оно работает только с младшими битами.

Нужно сделать так, чтобы '0' был 64-битным типом. Исправленный вариант:

letter_mask_ = ~(~0ull << flag_start_bit_);

Перечислю списком другие фрагменты кода, где осуществляется сдвиг отрицательных чисел:

dawg.cpp 188
intmatcher.cpp 172
intmatcher.cpp 174
intmatcher.cpp 176
intmatcher.cpp 178
intmatcher.cpp 180
intmatcher.cpp 182
intmatcher.cpp 184
intmatcher.cpp 186
intmatcher.cpp 188
intmatcher.cpp 190
intmatcher.cpp 192
intmatcher.cpp 194
intmatcher.cpp 196
intmatcher.cpp 198
intmatcher.cpp 200
intmatcher.cpp 202
intmatcher.cpp 323
intmatcher.cpp 347
intmatcher.cpp 366

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

TESSLINE* ApproximateOutline(....) {
  EDGEPT *edgept;
  ....
  edgept = edgesteps_to_edgepts(c_outline, edgepts);
  fix2(edgepts, area);
  edgept = poly2 (edgepts, area);  // 2nd approximation.
  ....
}

Предупреждение PVS-Studio: V519 The 'edgept' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 76, 78. polyaprx.cpp 78

Ещё один похожий случай:

inT32 row_words2(....)
{
  ....
  this_valid = blob_box.width () >= min_width;
  this_valid = TRUE;
  ....
}

Предупреждение PVS-Studio: V519 The 'this_valid' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 396, 397. wordseg.cpp 397

Неправильная последовательность инициализации членов класса

В начале рассмотрим класс 'MasterTrainer'. Обратите внимание, что член класса 'samples_' расположен до члена 'fontinfo_table_':

class MasterTrainer {
  ....
  TrainingSampleSet samples_;
  ....
  FontInfoTable fontinfo_table_;
  ....
};

Согласно стандарту порядок инициализация членов класса в конструкторе происходит в порядке их объявления в классе. Это значит, что 'samples_' будет инициализироваться ДО инициализации 'fontinfo_table_'.

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

MasterTrainer::MasterTrainer(NormalizationMode norm_mode,
                             bool shape_analysis,
                             bool replicate_samples,
                             int debug_level)
  : norm_mode_(norm_mode), samples_(fontinfo_table_),
    junk_samples_(fontinfo_table_),
    verify_samples_(fontinfo_table_),
    charsetsize_(0),
    enable_shape_anaylsis_(shape_analysis),
    enable_replication_(replicate_samples),
    fragments_(NULL), prev_unichar_id_(-1),
    debug_level_(debug_level)
{
}

Беда в том, что для инициализации 'samples_' используется ещё неинициализированная переменная 'fontinfo_table_'.

Аналогичная ситуация в этом классе с инициализацией полей 'junk_samples_' и 'verify_samples_'.

Я не берусь сказать, как лучше поступить с этим классом. Возможно будет достаточно перенести объявление 'fontinfo_table_' в самое начало класса.

Опечатка в условии

Опечатку заметить не просто, но анализатор не знает усталости.

class ScriptDetector {
  ....
  int korean_id_;
  int japanese_id_;
  int katakana_id_;
  int hiragana_id_;
  int han_id_;
  int hangul_id_;
  int latin_id_;
  int fraktur_id_;
  ....
};

void ScriptDetector::detect_blob(BLOB_CHOICE_LIST* scores) {
  ....
  if (prev_id == katakana_id_)
    osr_->scripts_na[i][japanese_id_] += 1.0;
  if (prev_id == hiragana_id_)
    osr_->scripts_na[i][japanese_id_] += 1.0;
  if (prev_id == hangul_id_)
    osr_->scripts_na[i][korean_id_] += 1.0;
  if (prev_id == han_id_)
    osr_->scripts_na[i][korean_id_] += kHanRatioInKorean;
  if (prev_id == han_id_)             <<<<====
    osr_->scripts_na[i][japanese_id_] += kHanRatioInJapanese;
  ....
}

Предупреждение PVS-Studio: V581 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 551, 553. osdetect.cpp 553

Скорее всего, самое последние сравнение должно быть таким:

if (prev_id == japanese_id_)

Ненужные проверки

Не нужно проверять, что возвращает оператор 'new'. Если не удастся выделить память, возникнет исключение. Конечно, можно сделать специальный оператор 'new', который возвращает нулевые указатели, но это отдельный случай (подробности).

В результате, вот такую функцию можно упростить:

void SetLabel(char_32 label) {
  if (label32_ != NULL) {
    delete []label32_;
  }
  label32_ = new char_32[2];
  if (label32_ != NULL) {
    label32_[0] = label;
    label32_[1] = 0;
  }
}

Предупреждение PVS-Studio: V668 There is no sense in testing the 'label32_' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. char_samp.h 73

Есть ещё 101 место, где проверяется указатель, который вернул оператор 'new'. Перечислять их в статье я не вижу смысла. Проще запустить для этого PVS-Studio.