search

SerenityOS / serenity

The Serenity Operating System 🐞
https://serenityos.org
BSD 2-Clause "Simplified" License
30.55k stars 3.19k forks source link

LibPDF: Regression: 4 PDFs in 0000.zip that used to open no longer do #25079

Open nico opened 1 week ago

nico commented 1 week ago

Meta/test_pdf.py ~/Downloads/0000 has these new crashes:

VERIFICATION FAILED: has_object(index) at /Users/thakis/src/serenity/Meta/Lagom/../../Userland/Libraries/LibPDF/XRefTable.h:81
0   liblagom-ak.0.0.0.dylib             0xc3ns0r3d ak_verification_failed + 216
1   liblagom-pdf.0.0.0.dylib            0xc3ns0r3d PDF::DocumentParser::initialize_non_linearized_xref_table() + 1676
2   liblagom-pdf.0.0.0.dylib            0xc3ns0r3d PDF::DocumentParser::initialize() + 564
3   liblagom-pdf.0.0.0.dylib            0xc3ns0r3d PDF::Document::create(AK::Span<unsigned char const>) + 220
4   pdf                                 0xc3ns0r3d pdf_main(Main::Arguments) + 568
5   pdf                                 0xc3ns0r3d serenity_main(Main::Arguments) + 64
6   pdf                                 0xc3ns0r3d main + 216
7   dyld                                0xc3ns0r3d start + 2360
In 4 files:
    /Users/thakis/Downloads/0000/0000567.pdf
    /Users/thakis/Downloads/0000/0000200.pdf
    /Users/thakis/Downloads/0000/0000900.pdf
    /Users/thakis/Downloads/0000/0000651.pdf
% cat run.sh
#!/bin/sh

Meta/serenity.sh build lagom pdf || exit 125

Build/lagom/bin/pdf --render out2.png ~/Downloads/0000/0000200.pdf || exit 1
% git bisect start
% git bisect bad 2fd718b7088762b2c573e59dbf7ac17ff89e61e1
% git bisect good 76ba374aef84932c21ff5a128b510664968c2eb3
% git bisect run ./run.sh
...
d458471e094f1c9ae3407eb2c6248278ae37cac3 is the first bad commit
commit d458471e094f1c9ae3407eb2c6248278ae37cac3
Author: Sergey Bugaev <bugaevc@gmail.com>
Date:   Wed Apr 24 17:02:51 2024 +0300

    LibPDF: Convert byte offsets to u64

    This fixes a build failure on 32-bit.

    Suggested-by: Nico Weber <thakis@chromium.org>

 Userland/Libraries/LibPDF/DocumentParser.cpp |  4 ++--
 Userland/Libraries/LibPDF/Function.cpp       |  2 +-
 Userland/Libraries/LibPDF/XRefTable.h        | 10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)
nico commented 2 days ago

linkify d458471e094f1c9ae3407eb2c6248278ae37cac3

ADKaster commented 2 days ago

curious that a "32-bit fix" commit could have that result. Is there something wrong with the check against the "invalid_offset" constant vs -1?