Skip to content

BUG: Use check_symbol_exists for TIFFFieldReadCount detection#6150

Merged
blowekamp merged 1 commit intoInsightSoftwareConsortium:release-5.4from
blowekamp:fix/tiff-field-readcount-detection
Apr 27, 2026
Merged

BUG: Use check_symbol_exists for TIFFFieldReadCount detection#6150
blowekamp merged 1 commit intoInsightSoftwareConsortium:release-5.4from
blowekamp:fix/tiff-field-readcount-detection

Conversation

@blowekamp
Copy link
Copy Markdown
Member

Summary

Fixes #6148.

When building with a system TIFF library (ITK_USE_SYSTEM_TIFF=ON), the CMake check for TIFFFieldReadCount used check_type_size(), which internally calls sizeof() on the argument. Passing a function name to sizeof() is non-standard — GCC and Clang accept it as an extension but MSVC rejects it — so ITK_TIFF_HAS_TIFFFieldReadCount was never defined on Windows builds using an external libTIFF.

Without that define, itkTIFFImageIO falls through to the libtiff 4.0.0–4.0.2 workaround that directly accesses the internal _TIFFField struct. That struct layout is no longer correct for modern libtiff versions, causing reads of garbage data and NULL pointer crashes when encountering tags with unsupported data types (e.g. an ICC profile tag).

Fix

Replace check_type_size(TIFFFieldReadCount ...) with check_symbol_exists(TIFFFieldReadCount "tiffio.h" ...), which correctly detects the function on all compilers including MSVC.

check_type_size on a function name fails on MSVC, causing crashes
with system TIFF on Windows. Fixes InsightSoftwareConsortium#6148.
@github-actions github-actions Bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:ThirdParty Issues affecting the ThirdParty module labels Apr 27, 2026
@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Apr 27, 2026

/azp run ITK.macOS

Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hjmjohnson
Copy link
Copy Markdown
Member

@greptile Review this PR.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

This PR fixes a CMake detection issue for TIFFFieldReadCount when building with a system TIFF library (ITK_USE_SYSTEM_TIFF=ON). The old check_type_size() call applied sizeof() to a function name — non-standard behavior that compiles on GCC/Clang as an extension but fails silently on MSVC, leaving ITK_TIFF_HAS_TIFFFieldReadCount undefined and causing downstream crashes with modern libtiff. The fix correctly replaces it with check_symbol_exists(), which is the proper CMake mechanism for detecting function symbols across all compilers.

Confidence Score: 5/5

Safe to merge — targeted, correct CMake fix with no functional regressions.

The change is minimal and well-reasoned: it swaps one CMake introspection call for the semantically correct one, adds the necessary module include, preserves all surrounding logic, and leaves the check_type_size(TIFFField* …) call (which operates on a real type) untouched. CMAKE_REQUIRED_LIBRARIES is already set before both checks. No issues were found.

No files require special attention.

Important Files Changed

Filename Overview
Modules/ThirdParty/TIFF/CMakeLists.txt Replaces check_type_size(TIFFFieldReadCount …) with check_symbol_exists(TIFFFieldReadCount "tiffio.h" …) and adds include(CheckSymbolExists); CMAKE_REQUIRED_LIBRARIES and the check_type_size(TIFFField* …) call are left intact and correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CMake configure with ITK_USE_SYSTEM_TIFF=ON] --> B[find_package TIFF::TIFF]
    B --> C[include CheckSymbolExists]
    C --> D["check_symbol_exists(TIFFFieldReadCount tiffio.h ...)"]
    D -->|Symbol found| E[ITK_TIFF_HAS_TIFFFieldReadCount = 1]
    D -->|Symbol not found| F[ITK_TIFF_HAS_TIFFFieldReadCount unset]
    E --> G[check_type_size TIFFField*]
    F --> G
    G --> H[configure_file itk_tiff.h.in → itk_tiff.h]
    H -->|ITK_TIFF_HAS_TIFFFieldReadCount defined| I[itkTIFFImageIO uses TIFFFieldReadCount API]
    H -->|Not defined| J[itkTIFFImageIO falls back to legacy _TIFFField struct access → crashes with modern libtiff]
Loading

Reviews (2): Last reviewed commit: "BUG: Use check_symbol_exists for TIFFFie..." | Re-trigger Greptile

blowekamp added a commit to blowekamp/libitk-feedstock that referenced this pull request Apr 27, 2026
When building with ITK_USE_SYSTEM_TIFF=ON, check_type_size() was used
to detect TIFFFieldReadCount (a function, not a type). Passing a function
to sizeof() is non-standard and silently fails on MSVC, leaving
ITK_TIFF_HAS_TIFFFieldReadCount undefined and causing itkTIFFImageIO to
fall through to a stale libtiff 4.0.0-4.0.2 workaround that accesses an
obsolete internal _TIFFField struct, causing NULL pointer crashes.

Fixes: InsightSoftwareConsortium/ITK#6148
Upstream PR: InsightSoftwareConsortium/ITK#6150
blowekamp added a commit to conda-forge/libitk-feedstock that referenced this pull request Apr 27, 2026
When building with ITK_USE_SYSTEM_TIFF=ON, check_type_size() was used
to detect TIFFFieldReadCount (a function, not a type). Passing a function
to sizeof() is non-standard and silently fails on MSVC, leaving
ITK_TIFF_HAS_TIFFFieldReadCount undefined and causing itkTIFFImageIO to
fall through to a stale libtiff 4.0.0-4.0.2 workaround that accesses an
obsolete internal _TIFFField struct, causing NULL pointer crashes.

Fixes: InsightSoftwareConsortium/ITK#6148
Upstream PR: InsightSoftwareConsortium/ITK#6150
@blowekamp blowekamp marked this pull request as ready for review April 27, 2026 19:09
@blowekamp blowekamp merged commit ccb17dc into InsightSoftwareConsortium:release-5.4 Apr 27, 2026
13 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ThirdParty Issues affecting the ThirdParty module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants