DanBloomberg / leptonica

Leptonica is an open source library containing software that is broadly useful for image processing and image analysis applications. The official github repository for Leptonica is: danbloomberg/leptonica. See leptonica.org for more documentation.
Other
1.74k stars 387 forks source link

Fix rankhisto_reg test by fixing square root of negative number #652

Closed stweil closed 1 year ago

stweil commented 1 year ago

The test fails in a debug build with compiler flag -ftrapv. That compiler flag modifies the code to detect illegal integer calculations.

An integer overflow was triggered because the difference of two floating point values was negative (rounding error) and the square root of negative values is undefined (NAN).

Converting NAN to integer results in an unexpected integer value.

Signed-off-by: Stefan Weil sw@weilnetz.de

stweil commented 1 year ago

I added two more commits which fix memory issues detected by the address sanitizer when running make check. Now all tests pass.

Instructions for reproduction (needs modified configure.ac which otherwise enforces wrong CFLAGS):

./configure '--disable-shared' '--enable-debug' 'CFLAGS=-D_GLIBCXX_DEBUG -DDEBUG=1 -Wall -pedantic -g -O0 -fsanitize=address,undefined -fstack-protector-strong -ftrapv'
make check
DanBloomberg commented 1 year ago

Thank you. I have used valgrind on the programs, but not the clang sanitizers.

Please say more about the "instructions for reproduction". We should have them written out so that anyone (like, me :-) can run sanitizers. What needs to happen to configure.ac?

stweil commented 1 year ago

Please say more about the "instructions for reproduction".

I'll try to create a GitHub action with sanitizers. Like that all steps are also documented and can be reproduced.

DanBloomberg commented 1 year ago

Thank you. I've added a section to the building part of the makefile. Also ran the sanitizer to find unused variables.

DanBloomberg commented 1 year ago

@stweil When the address sanitizer is run, there are many warnings, which make for a very noisy output.

The warnings are about variables that may be uninitialized (they seem ok), return values that are set but not used (needed to silence compilers and analyzers), static buffers made by the compiler that are larger than 4096 bytes (not a problem), variables that are not used because they're in a section of code that is not compiled, etc.

Because of all the noise, it's not clear to me how useful it would be as a github action. But I did 'document' your script in README.html

amitdo commented 1 year ago

These warning come from the -Wall -pedantic flags and not from the sanitizers flags.

DanBloomberg commented 1 year ago

If I configure without those two flags, and then run make clean I still get warnings about:

There are about 200 of these, essentially all "noise".