BUG: Prevent SIGSEGV in VectorImage SetPixel with shorter source VLV

[hjmjohnson]

Summary

Restore the ability to pass an under-sized itk::VariableLengthVector (including a length-0 one) as the source of a VectorImage pixel Set without crashing. Regression introduced by commit 1d87efa52 ("BUG: Make VariableLengthVector::m_Data null when its length is zero").

Root cause

Before 1d87efa52, VariableLengthVector<T>(length) always allocated via new T[length], so even a length-0 vector held a non-null (though zero-size) data pointer. DefaultVectorPixelAccessor::Set copies exactly m_VectorLength components from that source pointer:

for (VectorLengthType i = 0; i < m_VectorLength; ++i)
{
  truePixel[i] = input[i];
}

A source shorter than m_VectorLength was a latent caller bug, but "happened to work" because the non-null (out-of-spec) read did not crash. After 1d87efa52, a length-0 VLV has m_Data == nullptr, and the compiler-vectorized fixed-count copy becomes a memcpy(dst, nullptr, N*sizeof(T)) — SIGSEGV.

Reproducer (register state captured at crash):

#0  __memcpy_avx512_unaligned_erms
#1  <inlined in CreateAndInitializeImage<VectorImage<float,3>>>
rdi (dst) = 0x555555aa8be0     valid heap buffer
rsi (src) = 0x0                null
rdx (len) = 0xc (12 bytes)

Fix

Guard the copy loop in DefaultVectorPixelAccessor::Set so that at most min(m_VectorLength, input.GetSize()) components are copied. When the source is shorter, the trailing destination components are left unchanged. A length-0 source is now a safe no-op.

This preserves 1d87efa52's null-data intent for zero-length VLVs without requiring every internal copy path to check for null before dereferencing.

Regression test

Two new cases in itkVariableLengthVectorGTest.cxx:

• VectorImageSetPixelFromShorterSourceIsSafe — constructs a VLV<float>(0), sets it into a 3-component VectorImage<float,3> pixel via the iterator. Fails with SIGSEGV on current main, passes with this change.
• VectorImageSetPixelFromMatchedSourceWorks — the normal length-matched path; verifies the copy still works and round-trips.

How it was discovered

Hunting a SIGSEGV in InsightSoftwareConsortium/ITKPerformanceBenchmarking's CopyIterationBenchmark across the v5.4.5 → current main bisect range. The benchmark passes static_cast<PixelType>(count) to seed pixel values — for VectorImage, this calls the VLV length constructor, so the very first iteration produces a length-0 source. Before 1d87efa52 the benchmark silently worked (reading uninitialized bytes); after, it crashes.

The benchmark itself is independently being hardened in InsightSoftwareConsortium/ITKPerformanceBenchmarking#111 so it constructs correctly-sized VLV pixels and works across the full bisect range regardless of this ITK-side fix.

Test plan

• Added VectorImageSetPixelFromShorterSourceIsSafe reproducer; fails on main without the fix, passes with it
• Added VectorImageSetPixelFromMatchedSourceWorks for the happy path
• Locally rebuilt ITKCommon and re-ran CopyIterationBenchmark — 15/15 probes captured, rc=0
• CI

[greptile-apps]

Greptile Summary

This PR fixes a SIGSEGV in DefaultVectorPixelAccessor::Set triggered when a VariableLengthVector<T>(0) — whose m_Data is nullptr since commit 1d87efa52 — is passed as the source to a VectorImage pixel setter. The fix clamps the copy loop to min(m_VectorLength, input.GetSize()), making a zero-length (or any shorter) source a safe no-op while leaving the existing matched-length path completely unchanged. Two regression tests are added covering both the crash path and the normal round-trip.

Confidence Score: 5/5

Safe to merge — fix is minimal, targeted, and correct; all remaining findings are P2 style suggestions.

The code change is a one-liner loop-bound clamp with no API surface change. Both the crash path and the happy path are covered by new tests. All reviewer comments are P2 (debug diagnostic and test assertion) and do not block correctness.

No files require special attention.

Important Files Changed

Filename	Overview
Modules/Core/Common/include/itkDefaultVectorPixelAccessor.h	Fixes SIGSEGV by clamping the copy loop bound to min(m_VectorLength, input.GetSize()); change is minimal, correct, and safe for all existing callers.
Modules/Core/Common/test/itkVariableLengthVectorGTest.cxx	Adds two targeted regression tests: one for the crash path (zero-length source VLV) and one for the happy path (matched-length source); both test cases are well-structured and correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["it.Set(vlv) called"] --> B["DefaultVectorPixelAccessor::Set"]
    B --> C["copyLength = min(m_VectorLength, input.GetSize())"]
    C --> D{copyLength == 0?}
    D -- "Yes (e.g. VLV(0), m_Data=nullptr)" --> E["No-op: loop never executes\n✅ No SIGSEGV"]
    D -- "No (copyLength > 0)" --> F["Loop: truePixel[i] = input[i]\nfor i in [0, copyLength)"]
    F --> G{copyLength < m_VectorLength?}
    G -- "Yes (shorter source)" --> H["Trailing components left unchanged"]
    G -- "No (matched source)" --> I["All m_VectorLength components written\n✅ Normal path preserved"]

Loading

_{Reviews (1): Last reviewed commit: "BUG: Prevent SIGSEGV in VectorImage SetP..." | Re-trigger Greptile}

[dzenanz]

Fix looks good. I did not take a good look at the regression tests.

[N-Dekker]

Regression introduced by commit 1d87efa52 ("BUG: Make VariableLengthVector::m_Data null when its length is zero").

For the sake of clarity, I would like to mention that the default-constructor of VariableLengthVector already created a zero-length vector with null pointer inside, long before pull request #5713 commit 1d87efa

21 years ago:

ITK/Code/Common/itkVariableLengthVector.txx

Lines 26 to 32 in b516cb8

	/** Default constructor */
	template < typename TValueType >
	VariableLengthVector<TValueType >
	::VariableLengthVector() :
	m_LetArrayManageMemory( true ),
	m_Data( 0 ),
	m_NumElements( 0 ) {}

Note that 0 was used to null-initialize m_Data 🤓