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

Fix test failures on 32-bit platforms #2522

Closed
wants to merge 2 commits into from

Conversation

kevinbackhouse
Copy link
Collaborator

@kevinbackhouse kevinbackhouse commented Feb 23, 2023

These tests are failing because on 32-bit platforms they trigger an exception in a Safe::add<size_t>(), which is obviously easier to trigger on 32-bit than on 64-bit. This causes these files to produce a different error message on 32-bit platforms than on 64-bit. I have updated the tests to be less fussy about the exact contents of the error message.

@ghost
Copy link

ghost commented Feb 23, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@neheb
Copy link
Collaborator

neheb commented Feb 23, 2023

2: ======================================================================
2: FAIL: test_run (github.test_CVE_2017_11591.TestCvePoC)
2: ----------------------------------------------------------------------
2: Traceback (most recent call last):
2:   File "/exiv2/tests/system_tests.py", line 652, in test_run
2:     self.compare_stderr(i, command, processed_stderr, stderr)
2:   File "/exiv2/tests/system_tests.py", line 966, in check_no_ASAN_UBSAN_errors
2:     self.assertNotIn(UBSAN_MSG, got_stderr)
2: AssertionError: 'runtime error' unexpectedly found in 'Error: Directory Image, entry 0x0000 has invalid size 4286578688*8; skipping entry.\nWarning: Directory Image, entry 0x0111: Strip 0 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 1 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 2 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 3 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 4 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 5 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 7 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 8 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 10 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 12 is outside of the data area; ignored.\n/exiv2/src/tiffcomposite_int.cpp:298:45: runtime error: pointer index expression with base 0xf2100000 overflowed to 0x53100000\n'
2: 
2: ======================================================================
2: FAIL: test_run (github.test_CVE_2017_14859.TestCvePoC)
2: ----------------------------------------------------------------------
2: Traceback (most recent call last):
2:   File "/exiv2/tests/system_tests.py", line 652, in test_run
2:     self.compare_stderr(i, command, processed_stderr, stderr)
2:   File "/exiv2/tests/bugfixes/github/test_CVE_2017_14859.py", line 20, in compare_stderr
2:     self.assertIn(expected_stderr, got_stderr)
2: AssertionError: 'Exiv2 exception in print action for file /exiv2/test/data/005-invalid-mem:\ncorrupted image metadata\n' not found in 'Warning: Directory Image, entry 0x011a has unknown Exif (TIFF) type 64772; setting type size 1.\n/exiv2/src/tiffvisitor_int.cpp:1325:48: runtime error: pointer index expression with base 0xef802138 overflowed to 0x2f802140\n'
2: 
2: ----------------------------------------------------------------------
2: Ran 300 tests in 55.997s
2: 
2: FAILED (failures=2, skipped=10)

@codecov
Copy link

codecov bot commented Feb 23, 2023

Codecov Report

Merging #2522 (d29acfe) into main (230fbaf) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2522      +/-   ##
==========================================
- Coverage   64.66%   64.61%   -0.05%     
==========================================
  Files         103      103              
  Lines       22249    22232      -17     
  Branches    10859    10858       -1     
==========================================
- Hits        14388    14366      -22     
- Misses       5620     5627       +7     
+ Partials     2241     2239       -2     
Impacted Files Coverage Δ
src/tiffimage.cpp 70.92% <0.00%> (-1.03%) ⬇️
src/pentaxmn_int.cpp 71.86% <0.00%> (-0.87%) ⬇️
src/preview.cpp 46.80% <0.00%> (-0.86%) ⬇️
src/easyaccess.cpp 93.65% <0.00%> (-0.80%) ⬇️
src/exif.cpp 66.45% <0.00%> (-0.63%) ⬇️
src/types.cpp 93.90% <0.00%> (-0.56%) ⬇️
src/properties.cpp 75.45% <0.00%> (-0.44%) ⬇️
src/value.cpp 73.05% <0.00%> (-0.35%) ⬇️
include/exiv2/value.hpp 85.32% <0.00%> (-0.20%) ⬇️
src/tiffcomposite_int.hpp 90.09% <0.00%> (-0.18%) ⬇️
... and 10 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kevinbackhouse kevinbackhouse marked this pull request as draft February 25, 2023 15:01
@kevinbackhouse
Copy link
Collaborator Author

@neheb: I think this is ready now. Previously, I was trying to detect whether we're running on 32-bit or 64-bit and adjust the expected error message accordingly, but that didn't work on Windows. I couldn't figure it out, but I think maybe we're running a 32-bit python binary on Windows. But I discovered an additional problem, which is that some of our tests build a 32-bit exiv2, but run it on a 64-bit platform, so what would actually need to do is figure out whether the exiv2 binary is 32-bit or 64-bit. Instead, I just made these tests less fussy about the expected error message.

@kevinbackhouse kevinbackhouse marked this pull request as ready for review February 25, 2023 23:18
@neheb
Copy link
Collaborator

neheb commented Feb 25, 2023

I'd add the other commit to this PR:

curl https://github.com/Exiv2/exiv2/commit/8f6cb446a9e5474da0a329e9b438df2cf0f25175.patch | git am

@postscript-dev
Copy link
Collaborator

@kevinbackhouse

so what would actually need to do is figure out whether the exiv2 binary is 32-bit or 64-bit.

I don't know if it is helpful or not, but it is possible to find out the bit-length of the exiv2 app using

$ exiv2 --verbose --version --grep bits
exiv2 1.0.0.9
Bits=64

@neheb
Copy link
Collaborator

neheb commented Feb 26, 2023

: ======================================================================
2: FAIL: test_run (github.test_CVE_2017_11591.TestCvePoC)
2: ----------------------------------------------------------------------
2: Traceback (most recent call last):
2:   File "/exiv2/tests/system_tests.py", line 652, in test_run
2:     self.compare_stderr(i, command, processed_stderr, stderr)
2:   File "/exiv2/tests/system_tests.py", line 966, in check_no_ASAN_UBSAN_errors
2:     self.assertNotIn(UBSAN_MSG, got_stderr)
2: AssertionError: 'runtime error' unexpectedly found in 'Error: Directory Image, entry 0x0000 has invalid size 4286578688*8; skipping entry.\nWarning: Directory Image, entry 0x0111: Strip 0 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 1 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 2 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 3 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 4 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 5 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 7 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 8 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 10 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 12 is outside of the data area; ignored.\n/exiv2/src/tiffcomposite_int.cpp:298:45: runtime error: pointer index expression with base 0xf210a000 overflowed to 0x5310a000\n'

@kevinbackhouse
Copy link
Collaborator Author

@kevinbackhouse

so what would actually need to do is figure out whether the exiv2 binary is 32-bit or 64-bit.

I don't know if it is helpful or not, but it is possible to find out the bit-length of the exiv2 app using

$ exiv2 --verbose --version --grep bits
exiv2 1.0.0.9
Bits=64

Thanks @postscript-dev! I was wondering if we could do something like that.

@kevinbackhouse
Copy link
Collaborator Author

: ======================================================================
2: FAIL: test_run (github.test_CVE_2017_11591.TestCvePoC)
2: ----------------------------------------------------------------------
2: Traceback (most recent call last):
2:   File "/exiv2/tests/system_tests.py", line 652, in test_run
2:     self.compare_stderr(i, command, processed_stderr, stderr)
2:   File "/exiv2/tests/system_tests.py", line 966, in check_no_ASAN_UBSAN_errors
2:     self.assertNotIn(UBSAN_MSG, got_stderr)
2: AssertionError: 'runtime error' unexpectedly found in 'Error: Directory Image, entry 0x0000 has invalid size 4286578688*8; skipping entry.\nWarning: Directory Image, entry 0x0111: Strip 0 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 1 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 2 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 3 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 4 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 5 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 7 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 8 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 10 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 12 is outside of the data area; ignored.\n/exiv2/src/tiffcomposite_int.cpp:298:45: runtime error: pointer index expression with base 0xf210a000 overflowed to 0x5310a000\n'

Which platform is that failing on? I just switched on Windows x86 in this PR and everything is passing. (See commit 2: d29acfe)

@kevinbackhouse
Copy link
Collaborator Author

I've created a new PR that uses @postscript-dev's idea: #2527. Closing this one.

@neheb
Copy link
Collaborator

neheb commented Feb 26, 2023

: ======================================================================
2: FAIL: test_run (github.test_CVE_2017_11591.TestCvePoC)
2: ----------------------------------------------------------------------
2: Traceback (most recent call last):
2:   File "/exiv2/tests/system_tests.py", line 652, in test_run
2:     self.compare_stderr(i, command, processed_stderr, stderr)
2:   File "/exiv2/tests/system_tests.py", line 966, in check_no_ASAN_UBSAN_errors
2:     self.assertNotIn(UBSAN_MSG, got_stderr)
2: AssertionError: 'runtime error' unexpectedly found in 'Error: Directory Image, entry 0x0000 has invalid size 4286578688*8; skipping entry.\nWarning: Directory Image, entry 0x0111: Strip 0 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 1 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 2 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 3 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 4 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 5 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 7 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 8 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 10 is outside of the data area; ignored.\nWarning: Directory Image, entry 0x0111: Strip 12 is outside of the data area; ignored.\n/exiv2/src/tiffcomposite_int.cpp:298:45: runtime error: pointer index expression with base 0xf210a000 overflowed to 0x5310a000\n'

Which platform is that failing on? I just switched on Windows x86 in this PR and everything is passing. (See commit 2: d29acfe)

Docker command above. Well, with sanitizers enabled.

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

Successfully merging this pull request may close these issues.

3 participants