build: instrument PDFWriter + bundled deps when fuzzing is enabled - #406
Merged
Conversation
BUILD_FUZZING_HARNESS was declared inside an
if(PROJECT_IS_TOP_LEVEL AND EXISTS PDFWriterTesting) block near the end
of the top-level CMakeLists, well after every ADD_SUBDIRECTORY for the
library and bundled deps had already run. The option only affected the
harness target's own .cpp compile; PDFWriter and the bundled deps
(zlib/freetype/libpng/libjpeg/libtiff/libaesgm) were built without
sanitizer coverage instrumentation. libFuzzer reported just ~5
inline 8-bit counters per harness (the harness body), so
coverage-guided exploration was effectively off.
Move the option declaration to the top of the file (next to
PDFHUMMUS_SANITIZER) and add a global flag block:
if(BUILD_FUZZING_HARNESS)
add_compile_options(-fsanitize=fuzzer-no-link,address
-fno-omit-frame-pointer -g -O2)
add_link_options(-fsanitize=address)
endif()
-fsanitize=fuzzer-no-link instruments without linking the libFuzzer
runtime (which expects an LLVMFuzzerTestOneInput entry point that only
the harness binaries have).
In PDFWriterTesting/CMakeLists.txt, drop the redundant target-level
compile options (they're now global) and use target_link_options to
add -fsanitize=fuzzer (with runtime) + -fuse-ld=lld on the harness
binaries themselves.
Verified locally (macOS, brew llvm + lld):
- PDFParser harness: 5 → 11,489 instrumentation edges
23 → 2,791 seed-corpus coverage
- JPEG harness: 5 → 562 edges (was 3 cov, now 72)
dict actually pays off — 2,100 new units in 15s
- TIFF harness: 5 → 81,815 edges (full PDFWriter→libtiff→
libjpeg→zlib stack instrumented)
- Default build (BUILD_FUZZING_HARNESS=OFF): fresh-dir configure +
build clean, no behavior change
Contributor
There was a problem hiding this comment.
Pull request overview
Moves the BUILD_FUZZING_HARNESS CMake option to the top of the root CMakeLists.txt and adds global compile/link flags so that -fsanitize=fuzzer-no-link,address instrumentation is applied to PDFWriter and all bundled dependencies, not just the harness translation units. Previously the option was declared after the dependency ADD_SUBDIRECTORY calls, leaving libFuzzer with ~5 coverage edges and effectively disabling coverage-guided fuzzing.
Changes:
- Declare
BUILD_FUZZING_HARNESSearly and add globaladd_compile_options/add_link_optionsfor fuzzer + ASan instrumentation. - Remove the duplicate
option(...)declaration from the testing block (now just a pointer comment). - Simplify
PDFWriterTesting/CMakeLists.txtharness setup: drop per-target compile options, link with-fsanitize=fuzzer -fuse-ld=lld.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| CMakeLists.txt | Hoists BUILD_FUZZING_HARNESS option and applies global fuzzer/ASan flags before subdirectories. |
| PDFWriterTesting/CMakeLists.txt | Removes redundant per-target compile flags; uses target_link_options for fuzzer runtime + lld. |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the latent issue called out in #405's PR description:
BUILD_FUZZING_HARNESS=ONinstrumented only the harness's own translation unit, not the library or bundled deps. libFuzzer reported only ~5 inline 8-bit counters per harness (the harness body), so coverage-guided exploration was effectively off across all existing harnesses.Root cause
In the top-level
CMakeLists.txt,option(BUILD_FUZZING_HARNESS ...)was declared inside theif(PROJECT_IS_TOP_LEVEL AND EXISTS PDFWriterTesting)block near the end of the file — well after everyADD_SUBDIRECTORYforPDFWriterand the bundled deps had already configured. By the time CMake processed the harness target's compile flags, the library and deps were locked in with their default (uninstrumented) configuration.Compare with
PDFHUMMUS_SANITIZERat line 20: declared before the subdirectory adds, which is precisely why that flag does instrument bundled deps.Fix
option(BUILD_FUZZING_HARNESS ...)to the top of the top-levelCMakeLists.txt, alongsidePDFHUMMUS_SANITIZER-fsanitize=fuzzer-no-linkenables SanitizerCoverage instrumentation without linking the libFuzzer runtime (which expects anLLVMFuzzerTestOneInputentry point that only the harness binaries have).PDFWriterTesting/CMakeLists.txt, drop the now-redundant target-level compile options (they're inherited from the global add_compile_options) and usetarget_link_optionsto add-fsanitize=fuzzer(with runtime) +-fuse-ld=lldon the harness binaries themselves.Verification (macOS, brew llvm + lld)
Local fuzz-smoke results, before vs after:
PDFParserFuzzingHarnessJPEGImageParserFuzzingHarnessTIFFImageHandlerFuzzingHarnessThe TIFF count is highest because it goes through the full stack: PDFWriter → bundled libtiff → libjpeg → zlib, all now instrumented.
JPEG with the
jpeg.dictfrom #405 now produces real results: 2,100 new corpus units added in 15 seconds (was 17 in 60s previously). Coverage-guided exploration is finally on.Default build (
BUILD_FUZZING_HARNESS=OFF): fresh-dir configure + build clean. No effect on the standard build path.Composes with PDFHUMMUS_SANITIZER
Both touch global flags. Setting both — e.g.
-DBUILD_FUZZING_HARNESS=ON -DPDFHUMMUS_SANITIZER=undefined— composes cleanly: clang accepts the union and the harness gets fuzzer + ASan + UBSan all stacked.Test plan
-DBUILD_FUZZING_HARNESS=ONbuild with LLVM clang + lld, and the libFuzzer startup line for any harness shows a counter count in the thousands (not 5)./PDFParserFuzzingHarness Materials/fuzzing/MinimalFuzzingCorpusshows libFuzzer expanding the corpus over time (theNEWlines)BUILD_FUZZING_HARNESS=OFF) builds cleanly andcteststill passes-DBUILD_FUZZING_HARNESS=ON -DPDFHUMMUS_SANITIZER=undefinedbuilds and runs (compose smoke)