Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[debci] for 0.7.0 version #253

Open
picca opened this issue Aug 2, 2018 · 26 comments
Open

[debci] for 0.7.0 version #253

picca opened this issue Aug 2, 2018 · 26 comments

Comments

@picca
Copy link

picca commented Aug 2, 2018

Hello, it seems that there is some issue with the python3 implementation.

you can find the log here for all python[2-3][-dbg] version of fabio.

https://ci.debian.net/data/autopkgtest/unstable/amd64/p/python-fabio/751490/log.gz

cheers

@kif
Copy link
Member

kif commented Oct 11, 2018

We usually don't test the -dbg packages ...
I wonder if it makes sense to still produce them since we have now a package with symbols which is enough for debugging with GDB ?

@picca
Copy link
Author

picca commented Feb 9, 2021

Hello, I have this error message in the ci (still with the python dbg version):

https://ci.debian.net/packages/p/python-fabio/testing/amd64/

https://ci.debian.net/data/autopkgtest/testing/amd64/p/python-fabio/10371373/log.gz

python3.9-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.

First it seems that it is only triggered with the 1.19.5 version of numpy ???

If I look after this error message on internet, I have a bunch or relevant answer like this one.

inducer/pycuda#233

https://gitlab.tiker.net/inducer/pycuda/-/merge_requests/22/commits?commit_id=16c81fce32adb69fbb9ec70dbba75e040c0c211a

inducer/bpl-subset@0d99742...3702fb1#diff-f649bd645442d73d4a799580395b3c8a74f00780e8e1c37b9f0b66bdf21c2be5

I am wondering if this as some meaning for you...

the culprite could be in numpy 1.19.5, or a fabio bug triggered only with numpy 1.19.5 ...

@picca
Copy link
Author

picca commented Feb 9, 2021

after investigation the problem comes from these tests testSuite.addTest(codecs.suite())

@picca
Copy link
Author

picca commented Feb 9, 2021

in the codec this one is the wrong one... testSuite.addTest(test_tifimage.suite())

@picca
Copy link
Author

picca commented Feb 9, 2021

inside whcih we find this wrong test testsuite.addTest(loadTests(TestTif_LibTiffPic))

@picca
Copy link
Author

picca commented Feb 9, 2021

the fax2d.tif image seems to cause troubles ???

WARNING:fabio.tifimage:Unable to read /tmp/fabio_testdata_picca/libtiffpic.tar.gz__content/fax2d.tif with TiffIO due to Compressed TIFF images not supported except packbits, trying PIL
python3-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.

@picca
Copy link
Author

picca commented Feb 9, 2021

If I comment the code whcih use PIL

    def _read_with_pil(self, infile):
        # pilimage = PIL.Image.open(infile)                                                                                                                                              
        # header = self._read_header_from_pil(pilimage)                                                                                                                                  
        # data = pilutils.get_numpy_array(pilimage)                                                                                                                                      
        # frame = self._create_frame(data, header)                                                                                                                                       
        # self.header = frame.header                                                                                                                                                     
        # self.data = frame.data                                                                                                                                                         
        # self._shape = None                                                                                                                                                             
        # self.lib = "PIL"                                                                                                                                                               
	pass

the test pass. Indeed nothing is done.

@picca
Copy link
Author

picca commented Feb 9, 2021

this line seems to cause the trouble

        data = pilutils.get_numpy_array(pilimage)

@picca
Copy link
Author

picca commented Feb 9, 2021

By instrumenting the code, it seems that this image is the only one with a bool dtype

WARNING:fabio.tifimage:Unable to read /tmp/fabio_testdata_picca/libtiffpic.tar.gz__content/fax2d.tif with TiffIO due to Compressed TIFF images not supported except packbits, trying PIL
toto :  <class 'bool'>
python3-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.

@picca
Copy link
Author

picca commented Feb 9, 2021

a modification around the bool type was done in numpy

numpy/numpy#17918

@picca
Copy link
Author

picca commented Feb 9, 2021

ok, I can reproduce the bug with this small script

import numpy
import PIL.Image
img = PIL.Image.open("fax2d.tif")
numpy.asarray(img, bool)

python3-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.
Abandon

@picca
Copy link
Author

picca commented Feb 9, 2021

In order to try to understand something I ran it wit the normal Python

$ python3
Python 3.9.1+ (default, Feb  5 2021, 13:46:56) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import numpy
>>> import PIL.Image
>>> img = PIL.Image.open("fax2d.tif")
>>> numpy.asarray(img, bool)
array(True)
>>> img
<PIL.TiffImagePlugin.TiffImageFile image mode=1 size=1728x1082 at 0x7F0495798A30>
>>> numpy.asarray(img, numpy.bool)
array([[False, False, False, ..., False, False, False],
       [False, False, False, ..., False, False, False],
       [False, False, False, ..., False, False, False],
       ...,
       [False, False, False, ..., False, False, False],
       [False, False, False, ..., False, False, False],
       [False, False, False, ..., False, False, False]])

the result is different, if I use numpy.bool instead of bool. to me numpy.bool seems better with this version of numpy 1.19.5
If I remember correctly something changed around numpy.bool with numpy 1.20 :(. so be careful with this...

@picca
Copy link
Author

picca commented Feb 9, 2021

With the python3-dbg version, both case create troubles.

picca@2a02-8420-6c55-6500-d012-4688-0bee-a0c6:/tmp/fabio_testdata_picca/libtiffpic.tar.gz__content$ python3-dbg
Python 3.9.1+ (default, Feb  5 2021, 13:46:56) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import numpy
>>> import PIL.Image
>>> img = PIL.Image.open("fax2d.tif")
>>> numpy.asarray(img, numpy.bool)
python3-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.
Abandon
picca@2a02-8420-6c55-6500-d012-4688-0bee-a0c6:/tmp/fabio_testdata_picca/libtiffpic.tar.gz__content$ python3-dbg
Python 3.9.1+ (default, Feb  5 2021, 13:46:56) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import PIL.Image
>>> img = PIL.Image.open("fax2d.tif")
>>> import numpy
>>> numpy.asarray(img, bool)
python3-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.
Abandon

@picca
Copy link
Author

picca commented Feb 9, 2021

Still playing with all this... I will open the file with PIL, convert it to "F" and see if this is better.

$ python3-dbg
Python 3.9.1+ (default, Feb  5 2021, 13:46:56) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import PIL.Image
>>> img = PIL.Image.open("fax2d.tif")
>>> img
<PIL.TiffImagePlugin.TiffImageFile image mode=1 size=1728x1082 at 0x7F292454F280>
>>> img2 = PIL.Image.open("fax2d.tif")
>>> img.convert("F")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3/dist-packages/PIL/Image.py", line 904, in convert
    self.load()
  File "/usr/lib/python3/dist-packages/PIL/TiffImagePlugin.py", line 1088, in load
    return self._load_libtiff()
  File "/usr/lib/python3/dist-packages/PIL/TiffImagePlugin.py", line 1192, in _load_libtiff
    raise OSError(err)
OSError: -9
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A116E0>
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A11410>
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A110F0>
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A11320>
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A11690>
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A116E0>
>>> 

amazing, the first attempt to convert the image produce an OSError (-9), then the next attempt works.
I think that I am on something...

@picca
Copy link
Author

picca commented Feb 9, 2021

this error -9 come from the decoder C part of Pillow the meaning is

/* Errcodes */
#define IMAGING_CODEC_END        1
#define IMAGING_CODEC_OVERRUN   -1
#define IMAGING_CODEC_BROKEN    -2
#define IMAGING_CODEC_UNKNOWN   -3
#define IMAGING_CODEC_CONFIG    -8
#define IMAGING_CODEC_MEMORY    -9

@picca
Copy link
Author

picca commented Feb 9, 2021

we can find this error message here

$ rgrep IMAGING_CODEC_MEMORY
libImaging/XbmEncode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipEncode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipEncode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipEncode.c:                state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipEncode.c:                        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/GifEncode.c:            state->errcode = IMAGING_CODEC_MEMORY;\
libImaging/GifEncode.c:                        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/SgiRleDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/SgiRleDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/SgiRleDecode.c:        err = IMAGING_CODEC_MEMORY;
libImaging/Jpeg2KDecode.c:                state->errcode = IMAGING_CODEC_MEMORY;
libImaging/Imaging.h:#define IMAGING_CODEC_MEMORY    -9
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:                state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipDecode.c:                state->errcode = IMAGING_CODEC_MEMORY;

@picca
Copy link
Author

picca commented Feb 9, 2021

Running the code in gdb, I found that the MEMORY error comes from the _decodeStrip method

int _decodeStrip(Imaging im, ImagingCodecState state, TIFF *tiff) {
    INT32 strip_row;
    UINT8 *new_data;
    UINT32 rows_per_strip, row_byte_size;
    int ret;

    ret = TIFFGetField(tiff, TIFFTAG_ROWSPERSTRIP, &rows_per_strip);
    if (ret != 1) {
        rows_per_strip = state->ysize;
    }
    TRACE(("RowsPerStrip: %u \n", rows_per_strip));

    // We could use TIFFStripSize, but for YCbCr data it returns subsampled data size
    row_byte_size = (state->xsize * state->bits + 7) / 8;

    /* overflow check for realloc */
    if (INT_MAX / row_byte_size < rows_per_strip) {
        state->errcode = IMAGING_CODEC_MEMORY;                         <-------------   this one
        return -1;
    }

@picca
Copy link
Author

picca commented Feb 9, 2021

After the call of TIFFGetField(tiff, TIFFTAG_ROWSPERSTRIP, &rows_per_strip);

(gdb) p rows_per_strip 
$7 = 4294967295

the value of rows_per_strip seems strange

(gdb) p row_byte_size 
$1 = 216
(gdb) p state->xsize
$2 = 1728
(gdb) p state->bits
$3 = 1

@picca
Copy link
Author

picca commented Feb 9, 2021

during the compilation of Pillow, I can see these warning for the TiffDecode.c

src/libImaging/TiffDecode.c: In function ‘_decodeStripYCbCr’:
src/libImaging/TiffDecode.c:213:22: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uint32’ {aka ‘unsigned int’} [-Wsign-compare]
  213 |     if (state->xsize != img.width || state->ysize != img.height) {
      |                      ^~
src/libImaging/TiffDecode.c:213:51: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uint32’ {aka ‘unsigned int’} [-Wsign-compare]
  213 |     if (state->xsize != img.width || state->ysize != img.height) {
      |                                                   ^~
src/libImaging/TiffDecode.c: In function ‘ImagingLibTiffDecode’:
src/libImaging/TiffDecode.c:498:41: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
  498 |                 for (tile_y = 0; tile_y < current_tile_length; tile_y++) {
      |                                         ^

@picca
Copy link
Author

picca commented Feb 10, 2021

to my opinion getting rid of PIL would be nice and replace it with wand.

I could read the image and convert it with no error

import wand.image

img = wand.image.Image(filename="fax2d.tif")
arr = numpy.array(img)
>>> arr.shape
(1082, 1728, 1)

See you :))

@vallsv
Copy link
Contributor

vallsv commented Feb 10, 2021

This issue should be reported to the pillow project then, no?

We probably dont care about bool image at synchrotron so this image could be skipped from our tests (while it is not fixed in pillow).

@picca
Copy link
Author

picca commented Feb 10, 2021

for mask bool images are great :)).

Maybe removing all the specific code for tiff images, should help reduce the maintenance of fabio :)).
I agree that this bug report should be reported to pillow upstream. It was late, and I was fed-up. I was in a hurry in order to salvage fabio from exclusion of the next Debian stable release.

cheers

Fred

@kif
Copy link
Member

kif commented Feb 10, 2021

Thanks a lot Fred for this investigation.
I agree with the bug should be reported to Pillow
Bool images are often used to store masks ...

@kif
Copy link
Member

kif commented Feb 10, 2021

My initial comment remains: do we need to maintain the dbg version if nobody uses them ?

@picca
Copy link
Author

picca commented Feb 10, 2021

Yes, I think, because this trigger real bugs :)), it is nice to have this run in order to show leak of ressources or other probles :)

I have a doubt about this bug, because it seems that it was not present with numpy 1.19.4, but has I said previously, the output of the convert method is strange depending on the dtype.

The C code of Pillow, its not simple especillay if you consider CVE. at least imagemagick has a good decurity support :).

@kif
Copy link
Member

kif commented Feb 10, 2021

We could convert binary files into uint8 dtype, that would be easy on out side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants