MModuleSaverMeasurementsFITS: guard ring veto written as 1, memory corruption for L2 events with more than 10 hits, existing file appended instead of overwritten

@zoglauer

This issue was written by Claude under the direction of @zoglauer.

The unit test UTNModuleSaverMeasurementsFITS (PR #247) found three problems in MModuleSaverMeasurementsFITS.

1. The VETO column does not follow its own definition

The column is defined as "0=none, 1=hard ACD veto, 2=soft ACD veto, 3=guard ring veto":

nuclearizer/src/MModuleSaverMeasurementsFITS.cxx

Line 218 in 988005f

    
           {"VETO",          "1B",         "",    "Veto flag: 0=none, 1=hard ACD veto, 2=soft ACD veto, 3=guard ring veto",              true },

but the saver writes Event->IsVeto() ? 1 : 0:

nuclearizer/src/MModuleSaverMeasurementsFITS.cxx

Line 362 in 988005f

uint8_t veto = Event->IsVeto() ? 1 : 0;

IsVeto() is true for the shield veto and for the guard ring veto, so a guard ring veto is written as 1 (hard ACD veto) instead of 3, and both cannot be distinguished. Hard vs. soft ACD veto would also need a source in MReadOutAssembly.

2. L2 events with more than 10 hits write out of bounds

For L2 the hit arrays have the fixed length 10 (arrayLen), but the loop runs over all numHits:

nuclearizer/src/MModuleSaverMeasurementsFITS.cxx

Lines 365 to 376 in 988005f

    
           // L2: fixed-length 10 hit arrays, zero-padded  
        
           const unsigned int L2_HIT_LEN = 10; 
        
           bool isL2 = (m_OutputDataLevel == 2); 
        
           unsigned int arrayLen = isL2 ? L2_HIT_LEN : numHits; 
        
           // Fixed-length arrays for event-level data (initialize to zeros) 
        
           std::valarray<float> statTest(0.0f, 8);   // new spec changed to 8E 
        
           std::valarray<float> recoilDir(0.0f, 3); 
        
           std::valarray<float> recoilDirErr(0.0f, 3); 
        
           // Hit-level arrays sized to arrayLen (numHits for L1b, fixed 10 for L2) 
        
           std::valarray<uint8_t> seqHitArr((uint8_t)0, arrayLen);

nuclearizer/src/MModuleSaverMeasurementsFITS.cxx

Lines 425 to 465 in 988005f

    
             std::replace(quality_flag.begin(), quality_flag.end(), '\n', ';'); 
        
             if (quality_flag.length() > 64) { 
        
               quality_flag = quality_flag.substr(0, 64); 
        
             } 
        
           } 
        
           // check if PE is Compton event, and the PE-> GetNHits() is the same as the Event->GetNHits() 
        
           bool comptonEvent = (PE != nullptr && PE->GetType() == MPhysicalEvent::c_Compton && PE->GetNHits() == numHits); 
        
           for (unsigned int i = 0; i < numHits; ++i) { 
        
             // For Compton events, get hits from MPhysicalEventHit 
        
             // For photo / unreconstructed events, get hits from MHit 
        
             if (comptonEvent) { 
        
               const MPhysicalEventHit& hit = PE->GetHit(i); 
        
               MVector position = hit.GetPosition(); 
        
               x[i] = (float)position.X(); 
        
               y[i] = (float)position.Y(); 
        
               z[i] = (float)position.Z(); 
        
               MVector positionUncertainty = hit.GetPositionUncertainty(); 
        
               x_err[i] = (float)positionUncertainty.X(); 
        
               y_err[i] = (float)positionUncertainty.Y(); 
        
               z_err[i] = (float)positionUncertainty.Z(); 
        
               energy[i] = (float)hit.GetEnergy(); 
        
               energy_err[i] = (float)hit.GetEnergyUncertainty(); 
        
               seqHitArr[i] = (uint8_t)(i + 1);  
        
             } else { 
        
               MHit* hit = Event->GetHit(i); 
        
               MVector position = hit->GetPosition(); 
        
               x[i] = (float)position.X(); 
        
               y[i] = (float)position.Y(); 
        
               z[i] = (float)position.Z(); 
        
               MVector positionResolution = hit->GetPositionResolution(); 
        
               x_err[i] = (float)positionResolution.X(); 
        
               y_err[i] = (float)positionResolution.Y(); 
        
               z_err[i] = (float)positionResolution.Z();

An event with more than 10 hits writes past the end of the std::valarrays (heap corruption); in the test, an L2 event with 40 hits crashes the process. NUMHIT (40) would also not match the 10-element arrays. Such events need a defined handling (e.g. skip them and count them as skipped, or truncate with a quality flag).

3. An existing output file is appended to instead of overwritten

The comment says "overwrite if exists", but new FITS(FileName, RWmode::Write) opens an existing file; CCfits only overwrites with a leading ! in the file name:

nuclearizer/src/MModuleSaverMeasurementsFITS.cxx

Lines 182 to 183 in 988005f

    
           // Create new FITS file (overwrite if exists) 
        
           m_FITSFile = new FITS(string(FileName), RWmode::Write);

In the test, a second run into the same file (3 events, then 1 event) leaves 4 rows, and the first row is still from the first run. Results of an old run silently remain in the new file.

Done when: all problems described in this issue are fixed, all ExpectedToFail() instances related to this issue are removed from the unit tests, and the unit tests pass on both macOS and Linux.

Hi Andreas, for the VETO column, I currently use Event->GetShieldVeto() to identify shield vetoes and Event->GetGuardRingVeto() to identify guard-ring vetoes. However, I’m not sure how to distinguish between hard and soft ACS vetoes. Is there an existing method or flag for this, or would we need to add one? Could you also clarify what distinguishes the two and where that information is available? Thanks!

Navigation Menu

Repository navigation

MModuleSaverMeasurementsFITS: guard ring veto written as 1, memory corruption for L2 events with more than 10 hits, existing file appended instead of overwritten #270

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	// L2: fixed-length 10 hit arrays, zero-padded
	const unsigned int L2_HIT_LEN = 10;
	bool isL2 = (m_OutputDataLevel == 2);
	unsigned int arrayLen = isL2 ? L2_HIT_LEN : numHits;

	// Fixed-length arrays for event-level data (initialize to zeros)
	std::valarray<float> statTest(0.0f, 8); // new spec changed to 8E
	std::valarray<float> recoilDir(0.0f, 3);
	std::valarray<float> recoilDirErr(0.0f, 3);

	// Hit-level arrays sized to arrayLen (numHits for L1b, fixed 10 for L2)
	std::valarray<uint8_t> seqHitArr((uint8_t)0, arrayLen);


	std::replace(quality_flag.begin(), quality_flag.end(), '\n', ';');

	if (quality_flag.length() > 64) {
	quality_flag = quality_flag.substr(0, 64);
	}
	}

	// check if PE is Compton event, and the PE-> GetNHits() is the same as the Event->GetNHits()
	bool comptonEvent = (PE != nullptr && PE->GetType() == MPhysicalEvent::c_Compton && PE->GetNHits() == numHits);

	for (unsigned int i = 0; i < numHits; ++i) {
	// For Compton events, get hits from MPhysicalEventHit
	// For photo / unreconstructed events, get hits from MHit
	if (comptonEvent) {
	const MPhysicalEventHit& hit = PE->GetHit(i);
	MVector position = hit.GetPosition();
	x[i] = (float)position.X();
	y[i] = (float)position.Y();
	z[i] = (float)position.Z();

	MVector positionUncertainty = hit.GetPositionUncertainty();
	x_err[i] = (float)positionUncertainty.X();
	y_err[i] = (float)positionUncertainty.Y();
	z_err[i] = (float)positionUncertainty.Z();

	energy[i] = (float)hit.GetEnergy();
	energy_err[i] = (float)hit.GetEnergyUncertainty();

	seqHitArr[i] = (uint8_t)(i + 1);
	} else {
	MHit* hit = Event->GetHit(i);
	MVector position = hit->GetPosition();
	x[i] = (float)position.X();
	y[i] = (float)position.Y();
	z[i] = (float)position.Z();

	MVector positionResolution = hit->GetPositionResolution();
	x_err[i] = (float)positionResolution.X();
	y_err[i] = (float)positionResolution.Y();
	z_err[i] = (float)positionResolution.Z();

	// Create new FITS file (overwrite if exists)
	m_FITSFile = new FITS(string(FileName), RWmode::Write);

MModuleSaverMeasurementsFITS: guard ring veto written as 1, memory corruption for L2 events with more than 10 hits, existing file appended instead of overwritten #270

Description

Activity

mwy-wing commented on Oct 29, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions