Signed integers displayed as unsigned where `long` is 64-bit (typical on Linux and macOS)

[generalmimon]

Problem

I noticed that on Linux x86-64, the values of signed integers in the oFFs chunk are incorrectly displayed as unsigned (the input file was pngtest.png from libpng):

libpng-pngtest.txt:32

  chunk oFFs at offset 0x000fa, length 9: 4294967286x20 micrometers offset

Note

The oFFs chunk is a registered extension to the PNG spec, see https://w3c.github.io/png/extensions/Overview.html#C.oFFs.

On Windows 64-bit, however, the signed value is displayed correctly:

  chunk oFFs at offset 0x000fa, length 9: -10x20 micrometers offset

This is where the part after the colon : is printed in the pngcheck source code:

pngcheck/pngcheck.c

Lines 2412 to 2413 in bd33ad6

	printf(": %ldx%ld %s offset\n", LG(buffer), LG(buffer+4),
	(buffer[8] == 0)? "pixels":"micrometers");

The definition of the LG(p) macro is here:

pngcheck/pngcheck.c

Lines 182 to 186 in bd33ad6

	#define SH(p) ((ush)(uch)((p)[1]) | ((ush)(uch)((p)[0]) << 8))
	#define LG(p) ((ulg)(SH((p)+2)) | ((ulg)(SH(p)) << 16))
	#define SSH(p) ((ssh)(uch)((p)[1]) | ((ssh)(sch)((p)[0]) << 8))
	#define SLG(p) ((slg)(SH((p)+2)) | ((slg)(SSH(p)) << 16))

The type aliases are declared here:

pngcheck/pngcheck.c

Lines 138 to 143 in bd33ad6

	typedef unsigned char uch;
	typedef unsigned short ush;
	typedef unsigned long ulg;
	typedef signed char sch;
	typedef signed short ssh;
	typedef signed long slg;

You can see that the LG(p) macro deals only with unsigned types, so it reads an unsigned 32-bit integer as an unsigned long. The difference in behavior between 64-bit Linux and Windows apparently boils down to the fact that the long type is 32-bit on Windows, but 64-bit on Linux (see https://en.cppreference.com/c/language/arithmetic_types#Data_models).

Possible fix

At first glance, I thought the fix was to simply use SLG(p) instead, which returns signed long:

diff --git i/pngcheck.c w/pngcheck.c
index d450790..fac8278 100644
--- i/pngcheck.c
+++ w/pngcheck.c
@@ -2409,7 +2409,7 @@ FIXME: make sure bit 31 (0x80000000) is 0
         set_err(kMinorError);
       }
       if (verbose && no_err(kMinorError)) {
-        printf(": %ldx%ld %s offset\n", LG(buffer), LG(buffer+4),
+        printf(": %ldx%ld %s offset\n", SLG(buffer), SLG(buffer+4),
                (buffer[8] == 0)? "pixels":"micrometers");
       }
       have_oFFs = 1;

However, after looking at the implementation of the macros SLG(p) and SSH(p) (the latter is used by the former), I was concerned that it might invoke undefined behavior:

pngcheck/pngcheck.c

Lines 185 to 186 in bd33ad6

	#define SSH(p) ((ssh)(uch)((p)[1]) | ((ssh)(sch)((p)[0]) << 8))
	#define SLG(p) ((slg)(SH((p)+2)) | ((slg)(SSH(p)) << 16))

This is because I vaguely remembered that in C, bit shifts are usually performed only on unsigned integers, because shifting signed integers is not always well-defined. To check whether there is any undefined behavior, I told Claude to edit CMakeLists.txt to enable UndefinedBehaviorSanitizer (UBSan). This was the result:

diff --git c/CMakeLists.txt i/CMakeLists.txt
index e59f4a8..74a058c 100644
--- c/CMakeLists.txt
+++ i/CMakeLists.txt
@@ -21,6 +21,37 @@ endif()
 # Executables
 add_executable(pngcheck ${PNGCHECK_SOURCES})

+# UndefinedBehaviorSanitizer support
+option(PNGCHECK_ENABLE_UBSAN "Build pngcheck with UndefinedBehaviorSanitizer instrumentation" OFF)
+if(PNGCHECK_ENABLE_UBSAN)
+  if(NOT (CMAKE_C_COMPILER_ID STREQUAL "GNU" OR CMAKE_C_COMPILER_ID MATCHES "Clang"))
+    message(FATAL_ERROR "PNGCHECK_ENABLE_UBSAN requires GCC or Clang")
+  endif()
+
+  # -fsanitize=undefined: the "strict UB" bundle. Per Clang's UBSan docs
+  # this already EXCLUDES everything that is merely
+  # suspicious but well-defined: unsigned-integer-overflow, the
+  # implicit-conversion group, the nullability-* group, and
+  # float-divide-by-zero (defined by IEEE 754).
+  #
+  # -O0 avoids optimizations (inlining, value tracking, UB-based code
+  # elision) that can hide or relocate the exact spot where UB happens.
+  # -g and -fno-omit-frame-pointer give accurate, symbolized stack traces.
+  # -fno-sanitize-recover means pngcheck prints a report and exits on the
+  # FIRST violation instead of continuing to run on corrupted state, so
+  # each run points at exactly one bug.
+  set(PNGCHECK_UBSAN_FLAGS
+      -fsanitize=undefined
+      -fno-sanitize-recover=undefined
+      -fno-omit-frame-pointer
+      -g
+      -O0
+  )
+
+  target_compile_options(pngcheck PRIVATE ${PNGCHECK_UBSAN_FLAGS})
+  target_link_options(pngcheck PRIVATE -fsanitize=undefined -g)
+endif()
+
 # Dependency handling
 if(PNGCHECK_USE_SYSTEM_ZLIB)
   find_package(ZLIB)

Then I built the project (note: I used Clang, but you can use GCC instead if you want; it might even be better, because then you only need to install the gcc cmake make zlib1g-dev packages, which take up only 290 MB of space):

pp@DESKTOP-89OPGF3:~/ks-experiments/pngcheck-bug$ podman run --pull=never --rm -it -v "$(pwd):/share" --workdir /share --security-opt no-new-privileges --cap-drop all --cap-add SETUID --cap-add SETGID --cap-add CHOWN --cap-add FOWNER --cap-add DAC_OVERRIDE docker.io/library/ubuntu:resolute-20260707
root@d165e6782a42:/share# apt update
(...)
root@d165e6782a42:/share# apt install --no-install-recommends clang libclang-rt-dev cmake make zlib1g-dev
Installing:
  clang  cmake  libclang-rt-dev  make  zlib1g-dev

Installing dependencies:
  (...)

Suggested packages:
  (...)

Recommended packages:
  (...)

Summary:
  Upgrading: 0, Installing: 74, Removing: 0, Not Upgrading: 5
  Download size: 111 MB
  Space needed: 517 MB / 1012 GB available

Continue? [Y/n] y
(...)
root@d165e6782a42:/share# cd pngcheck/
root@d165e6782a42:/share/pngcheck# mkdir build && cd build
root@d165e6782a42:/share/pngcheck/build# cmake .. -DPNGCHECK_ENABLE_UBSAN=ON -DCMAKE_BUILD_TYPE=Debug
-- The C compiler identification is Clang 21.1.8
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Found ZLIB: /usr/lib/x86_64-linux-gnu/libz.so (found version "1.3.1")
-- Configuring done (0.4s)
-- Generating done (0.0s)
-- Build files have been written to: /share/pngcheck/build
root@d165e6782a42:/share/pngcheck/build# cmake --build .
[ 50%] Building C object CMakeFiles/pngcheck.dir/pngcheck.c.o
[100%] Linking C executable pngcheck
[100%] Built target pngcheck
root@d165e6782a42:/share/pngcheck/build# ./pngcheck -cvt ../pngtest.png
File: ../pngtest.png (8831 bytes)
  chunk IHDR at offset 0x0000c, length 13
    91 x 69 image, 32-bit RGB+alpha, interlaced
  chunk sBIT at offset 0x00025, length 4
    red = 5 = 0x05, green = 5 = 0x05, blue = 5 = 0x05, alpha = 5 = 0x05
  chunk cLLI at offset 0x00035, length 8
    Maximum content light level = 300 cd/m^2
    Maximum frame average light level = 20 cd/m^2
  chunk mDCV at offset 0x00049, length 24
    Mastering Display
    White x = 0.17284 y = 0.17926,  Red x = 0.36734 y = 0.13264
    Green x = 0.0798 y = 0.4202,  Blue x = 0.0183 y = 4e-05
    Maximum luminance = 80 cd/m^2
    Minimum luminance = 1 cd/m^2
  chunk cICP at offset 0x0006d, length 4
   IEC 61966-2-1 sRGB
    White x = 0.3127 y = 0.329,  Red x = 0.64 y = 0.33
    Green x = 0.3 y = 0.6,  Blue x = 0.15 y = 0.06
    Full range
  chunk sRGB at offset 0x0007d, length 1
    rendering intent = relative colorimetric
  chunk gAMA at offset 0x0008a, length 4: 0.45455
  chunk cHRM at offset 0x0009a, length 32
    White x = 0.31270 y = 0.32900,  Red x = 0.64000 y = 0.33000
    Green x = 0.30000 y = 0.60000,  Blue x = 0.15000 y = 0.06000
  chunk sTER at offset 0x000c6, length 1
    stereo subimage layout = divergent (parallel)
  chunk vpAg at offset 0x000d3, length 9
    unknown private, ancillary, safe-to-copy chunk
  chunk bKGD at offset 0x000e8, length 6
    red = 0x00e0, green = 0x00e0, blue = 0x0080
/share/pngcheck/pngcheck.c:2412:41: runtime error: left shift of negative value -1
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /share/pngcheck/pngcheck.c:2412:41

Line 2412 is exactly the printf statement where I replaced 1) LG(buffer) with SLG(buffer) and 2) LG(buffer+4) with SLG(buffer+4):

$ cat -n pngcheck.c | sed -n '2411,$p;2414q'

  2411        if (verbose && no_err(kMinorError)) {
  2412          printf(": %ldx%ld %s offset\n", SLG(buffer), SLG(buffer+4),
  2413                 (buffer[8] == 0)? "pixels":"micrometers");
  2414        }

So UBSan confirmed my suspicion that the current implementation of the SLG(p) macro can invoke undefined behavior. This should be fixed before it can be used in the oFFs chunk.

The SSH(p) macro is used directly only in the SLG(p) macro. The SLG(p) macro is currently only used when parsing the cHRM chunk:

pngcheck/pngcheck.c

Lines 2030 to 2037 in bd33ad6

	wx = (double)SLG(buffer)/100000;
	wy = (double)SLG(buffer+4)/100000;
	rx = (double)SLG(buffer+8)/100000;
	ry = (double)SLG(buffer+12)/100000;
	gx = (double)SLG(buffer+16)/100000;
	gy = (double)SLG(buffer+20)/100000;
	bx = (double)SLG(buffer+24)/100000;
	by = (double)SLG(buffer+28)/100000;

However, according to the PNG Third Edition specification, these should be unsigned integers, not signed:

Each value is encoded as a PNG four-byte unsigned integer, representing the x or y value times 100000.

This mismatch doesn't matter much in practice, because the maximum valid value of a PNG unsigned integer is limited to 2**31 - 1 (i.e. 0x7fff_ffff):

PNG four-byte unsigned integer

a four-byte unsigned integer limited to the range 0 to 2³¹-1.

Values that are indeed within this range will be decoded correctly regardless of whether signed or unsigned integer parsing is used. Nevertheless, parsing as a signed integer when the specification states that it is unsigned is clearly an error that should be fixed.