Skip to content

Commit 8f3056b

Browse files
imikejacksonimikejackson
committed
BUG: Fix SIMPL JSON conversion segfault and re-enable backwards-compat CHECKs
* Fix Linux segfault in ReadH5EbsdFilter SIMPL conversion path. The custom ReadH5EbsdFilterParameterConverter was invoked via ConvertTopParameters, which passed the whole filter JSON to a converter that indexed missing top-level keys with const operator[]. Under NDEBUG this strips the nlohmann::json assertion and dereferences end(), producing UB that manifests as a segfault on Linux. Switch to ConvertParameter with SIMPL::k_ReadH5EbsdKey so the converter receives the inner JSON it expects, add a contains() precheck for all required keys, and accept both boolean and integer for UseTransformations. * Make SingleToMultiDataPathSelectionFilterParameterConverter tolerant of optional Attribute Matrix Name and Data Array Name fields so DataPaths of varying depth (DataContainer / AttributeMatrix / DataArray) all convert correctly. Real SIMPL serialized DataContainerSelection, AttributeMatrixSelection, and DataArraySelection parameters with the same JSON shape and progressively more fields; the converter now mirrors that. * Add Filter_Name fallback in MultiThresholdObjectsFilter::FromSIMPLJson so SIMPL 6.4 pipelines (which lack Filter_Uuid) route to the correct advanced vs v1 ComparisonSelection converter based on class name. * Repair fixtures generated with incorrect SIMPL formats: - InitRange object instead of "0;1" string - SelectedScanNames array instead of object - EnsembleInfo as array of phase objects with valid enum indices - ZSpacing values that match the test expectations * Re-enable previously commented-out CHECK statements across 11 backwards- compatibility test files. Where multiple CHECKs were stubbed for alternate fixture branches, keep only the ones that match the active fixture's UUID; expand coverage with additional CHECKs for path builders, vector parameters, and custom value types so the tests actually validate the converted Arguments rather than just confirming the pipeline loads. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
1 parent f854bb6 commit 8f3056b

23 files changed

Lines changed: 142 additions & 185 deletions

‎src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/ReadH5EbsdFilter.cpp‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,15 @@ struct ReadH5EbsdFilterParameterConverter
322322

323323
static Result<ValueType> convert(const nlohmann::json& json)
324324
{
325+
static constexpr std::array<StringLiteral, 6> requiredKeys = {k_InputFileKey, k_SelectedArrayNamesKey, k_ZStartIndexKey, k_ZEndIndexKey, k_RefFrameZDirKey, k_UseTransformationsKey};
326+
for(const auto& key : requiredKeys)
327+
{
328+
if(!json.contains(key.view()))
329+
{
330+
return MakeErrorResult<ValueType>(-3, fmt::format("H5EbsdReaderParameterConverter json is missing required key '{}'", key.view()));
331+
}
332+
}
333+
325334
if(!json[k_InputFileKey].is_string())
326335
{
327336
return MakeErrorResult<ValueType>(-2, fmt::format("H5EbsdReaderParameterConverter json '{}' is not a string", json[k_InputFileKey].dump()));
@@ -342,9 +351,9 @@ struct ReadH5EbsdFilterParameterConverter
342351
{
343352
return MakeErrorResult<ValueType>(-1, fmt::format("H5EbsdReaderParameterConverter json '{}' is not an integer", json[k_RefFrameZDirKey].dump()));
344353
}
345-
if(!json[k_UseTransformationsKey].is_number_integer())
354+
if(!json[k_UseTransformationsKey].is_number_integer() && !json[k_UseTransformationsKey].is_boolean())
346355
{
347-
return MakeErrorResult<ValueType>(-1, fmt::format("H5EbsdReaderParameterConverter json '{}' is not an integer", json[k_UseTransformationsKey].dump()));
356+
return MakeErrorResult<ValueType>(-1, fmt::format("H5EbsdReaderParameterConverter json '{}' is not an integer or boolean", json[k_UseTransformationsKey].dump()));
348357
}
349358

350359
for(const auto& iter : json[k_SelectedArrayNamesKey])
@@ -360,7 +369,7 @@ struct ReadH5EbsdFilterParameterConverter
360369
value.startSlice = json[k_ZStartIndexKey].get<int32>();
361370
value.endSlice = json[k_ZEndIndexKey].get<int32>();
362371
value.eulerRepresentation = json[k_RefFrameZDirKey].get<int32>() - 1;
363-
value.useRecommendedTransform = static_cast<bool>(json[k_UseTransformationsKey].get<int32>());
372+
value.useRecommendedTransform = json[k_UseTransformationsKey].is_boolean() ? json[k_UseTransformationsKey].get<bool>() : static_cast<bool>(json[k_UseTransformationsKey].get<int32>());
364373
value.selectedArrayNames = json[k_SelectedArrayNamesKey].get<std::vector<std::string>>();
365374

366375
return {std::move(value)};
@@ -375,7 +384,7 @@ Result<Arguments> ReadH5EbsdFilter::FromSIMPLJson(const nlohmann::json& json)
375384

376385
std::vector<Result<>> results;
377386

378-
results.push_back(SIMPLConversion::ConvertTopParameters<SIMPLConversionCustom::ReadH5EbsdFilterParameterConverter>(args, json, k_ReadH5EbsdParameter_Key));
387+
results.push_back(SIMPLConversion::ConvertParameter<SIMPLConversionCustom::ReadH5EbsdFilterParameterConverter>(args, json, SIMPL::k_ReadH5EbsdKey, k_ReadH5EbsdParameter_Key));
379388
results.push_back(SIMPLConversion::ConvertParameter<SIMPLConversion::DCPathBuilderFilterParameterConverter>(args, json, SIMPL::k_DataContainerNameKey, k_CreatedImageGeometryPath_Key));
380389
results.push_back(SIMPLConversion::ConvertParameter<SIMPLConversion::LinkedPathCreationFilterParameterConverter>(args, json, SIMPL::k_CellAttributeMatrixNameKey, k_CellAttributeMatrixName_Key));
381390
results.push_back(

‎src/Plugins/OrientationAnalysis/test/CreateEnsembleInfoTest.cpp‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -184,15 +184,16 @@ TEST_CASE("OrientationAnalysis::CreateEnsembleInfoFilter: SIMPL Backwards Compat
184184
REQUIRE(filter != nullptr);
185185
REQUIRE(filter->uuid() == FilterTraits<CreateEnsembleInfoFilter>::uuid);
186186

187-
// Note: Complex SIMPL parameter conversions may produce warnings
188-
// pipelineFilter->getComments() may not be empty for filters with custom converters
189-
190187
const Arguments args = pipelineFilter->getArguments();
191-
// Complex type (AMPathBuilderFilterParameterConverter) - verified by successful pipeline loading
192-
// Complex type (EnsembleInfoFilterParameterConverter) - verified by successful pipeline loading
193-
// CHECK(args.value<std::string>(CreateEnsembleInfoFilter::k_CrystalStructuresArrayName_Key) == "TestName");
194-
// CHECK(args.value<std::string>(CreateEnsembleInfoFilter::k_PhaseTypesArrayName_Key) == "TestName");
195-
// CHECK(args.value<std::string>(CreateEnsembleInfoFilter::k_PhaseNamesArrayName_Key) == "TestName");
188+
CHECK(args.value<std::string>(CreateEnsembleInfoFilter::k_CrystalStructuresArrayName_Key) == "TestName");
189+
CHECK(args.value<std::string>(CreateEnsembleInfoFilter::k_PhaseTypesArrayName_Key) == "TestName");
190+
CHECK(args.value<std::string>(CreateEnsembleInfoFilter::k_PhaseNamesArrayName_Key) == "TestName");
191+
CHECK(args.value<DataPath>(CreateEnsembleInfoFilter::k_CellEnsembleAttributeMatrixPath_Key) == DataPath({"DataContainer", "CellEnsembleData"}));
192+
const auto ensembleValue = args.value<EnsembleInfoParameter::ValueType>(CreateEnsembleInfoFilter::k_Ensemble_Key);
193+
REQUIRE(ensembleValue.size() == 1);
194+
CHECK(ensembleValue[0][0] == "Cubic-High m-3m");
195+
CHECK(ensembleValue[0][1] == "Primary");
196+
CHECK(ensembleValue[0][2] == "Primary");
196197
}
197198
}
198199
}

‎src/Plugins/OrientationAnalysis/test/ReadH5EbsdTest.cpp‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,17 @@ TEST_CASE("OrientationAnalysis::ReadH5EbsdFilter: SIMPL Backwards Compatibility"
188188
REQUIRE(filter != nullptr);
189189
REQUIRE(filter->uuid() == FilterTraits<ReadH5EbsdFilter>::uuid);
190190

191-
// Note: Complex SIMPL parameter conversions may produce warnings
192-
// pipelineFilter->getComments() may not be empty for filters with custom converters
193-
194191
const Arguments args = pipelineFilter->getArguments();
195-
// CHECK(args.value<DataPath>(ReadH5EbsdFilter::k_CreatedImageGeometryPath_Key) == DataPath({"DataContainer"}));
196-
// CHECK(args.value<std::string>(ReadH5EbsdFilter::k_CellAttributeMatrixName_Key) == "TestName");
197-
// CHECK(args.value<std::string>(ReadH5EbsdFilter::k_CellEnsembleAttributeMatrixName_Key) == "TestName");
192+
CHECK(args.value<DataPath>(ReadH5EbsdFilter::k_CreatedImageGeometryPath_Key) == DataPath({"DataContainer"}));
193+
CHECK(args.value<std::string>(ReadH5EbsdFilter::k_CellAttributeMatrixName_Key) == "TestName");
194+
CHECK(args.value<std::string>(ReadH5EbsdFilter::k_CellEnsembleAttributeMatrixName_Key) == "TestName");
195+
196+
const auto h5EbsdValue = args.value<ReadH5EbsdFileParameter::ValueType>(ReadH5EbsdFilter::k_ReadH5EbsdParameter_Key);
197+
CHECK(h5EbsdValue.inputFilePath == "/test/path/file.h5ebsd");
198+
CHECK(h5EbsdValue.startSlice == 0);
199+
CHECK(h5EbsdValue.endSlice == 10);
200+
CHECK(h5EbsdValue.useRecommendedTransform == true);
201+
CHECK(h5EbsdValue.selectedArrayNames == std::vector<std::string>{"Confidence Index", "Image Quality"});
198202
}
199203
}
200204
}

‎src/Plugins/OrientationAnalysis/test/ReadH5EspritDataTest.cpp‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -223,18 +223,16 @@ TEST_CASE("OrientationAnalysis::ReadH5EspritDataFilter: SIMPL Backwards Compatib
223223
REQUIRE(filter != nullptr);
224224
REQUIRE(filter->uuid() == FilterTraits<ReadH5EspritDataFilter>::uuid);
225225

226-
// Note: Complex SIMPL parameter conversions may produce warnings
227-
// pipelineFilter->getComments() may not be empty for filters with custom converters
228-
229226
const Arguments args = pipelineFilter->getArguments();
230-
// Complex type (OEMEbsdScanSelectionFilterParameterConverter) - verified by successful pipeline loading
231-
// CHECK(args.value<float32>(ReadH5EspritDataFilter::k_ZSpacing_Key) == 2.5f);
232-
// Complex type (FloatVec3FilterParameterConverter) - verified by successful pipeline loading
233-
// CHECK(args.value<bool>(ReadH5EspritDataFilter::k_DegreesToRadians_Key) == true);
234-
// CHECK(args.value<bool>(ReadH5EspritDataFilter::k_ReadPatternData_Key) == true);
235-
// CHECK(args.value<DataPath>(ReadH5EspritDataFilter::k_CreatedImageGeometryPath_Key) == DataPath({"DataContainer"}));
236-
// CHECK(args.value<std::string>(ReadH5EspritDataFilter::k_CellAttributeMatrixName_Key) == "TestName");
237-
// CHECK(args.value<std::string>(ReadH5EspritDataFilter::k_CellEnsembleAttributeMatrixName_Key) == "TestName");
227+
CHECK(args.value<float32>(ReadH5EspritDataFilter::k_ZSpacing_Key) == 2.5f);
228+
CHECK(args.value<bool>(ReadH5EspritDataFilter::k_DegreesToRadians_Key) == true);
229+
CHECK(args.value<bool>(ReadH5EspritDataFilter::k_ReadPatternData_Key) == true);
230+
CHECK(args.value<DataPath>(ReadH5EspritDataFilter::k_CreatedImageGeometryPath_Key) == DataPath({"DataContainer"}));
231+
CHECK(args.value<std::string>(ReadH5EspritDataFilter::k_CellAttributeMatrixName_Key) == "TestName");
232+
CHECK(args.value<std::string>(ReadH5EspritDataFilter::k_CellEnsembleAttributeMatrixName_Key) == "TestName");
233+
const auto scanSel = args.value<OEMEbsdScanSelectionParameter::ValueType>(ReadH5EspritDataFilter::k_SelectedScanNames_Key);
234+
CHECK(scanSel.inputFilePath == "/test/path/file.h5");
235+
CHECK(scanSel.scanNames == std::list<std::string>{"Scan1"});
238236
}
239237
}
240238
}

‎src/Plugins/OrientationAnalysis/test/ReadH5OimDataTest.cpp‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -214,17 +214,15 @@ TEST_CASE("OrientationAnalysis::ReadH5OimDataFilter: SIMPL Backwards Compatibili
214214
REQUIRE(filter != nullptr);
215215
REQUIRE(filter->uuid() == FilterTraits<ReadH5OimDataFilter>::uuid);
216216

217-
// Note: Complex SIMPL parameter conversions may produce warnings
218-
// pipelineFilter->getComments() may not be empty for filters with custom converters
219-
220217
const Arguments args = pipelineFilter->getArguments();
221-
// Complex type (OEMEbsdScanSelectionFilterParameterConverter) - verified by successful pipeline loading
222-
// CHECK(args.value<float32>(ReadH5OimDataFilter::k_ZSpacing_Key) == 2.5f);
223-
// Complex type (FloatVec3FilterParameterConverter) - verified by successful pipeline loading
224-
// CHECK(args.value<bool>(ReadH5OimDataFilter::k_ReadPatternData_Key) == true);
225-
// CHECK(args.value<DataPath>(ReadH5OimDataFilter::k_CreatedImageGeometryPath_Key) == DataPath({"DataContainer"}));
226-
// CHECK(args.value<std::string>(ReadH5OimDataFilter::k_CellAttributeMatrixName_Key) == "TestName");
227-
// CHECK(args.value<std::string>(ReadH5OimDataFilter::k_CellEnsembleAttributeMatrixName_Key) == "TestName");
218+
CHECK(args.value<float32>(ReadH5OimDataFilter::k_ZSpacing_Key) == 2.5f);
219+
CHECK(args.value<bool>(ReadH5OimDataFilter::k_ReadPatternData_Key) == true);
220+
CHECK(args.value<DataPath>(ReadH5OimDataFilter::k_CreatedImageGeometryPath_Key) == DataPath({"DataContainer"}));
221+
CHECK(args.value<std::string>(ReadH5OimDataFilter::k_CellAttributeMatrixName_Key) == "TestName");
222+
CHECK(args.value<std::string>(ReadH5OimDataFilter::k_CellEnsembleAttributeMatrixName_Key) == "TestName");
223+
const auto scanSel = args.value<OEMEbsdScanSelectionParameter::ValueType>(ReadH5OimDataFilter::k_SelectedScanNames_Key);
224+
CHECK(scanSel.inputFilePath == "/test/path/file.h5");
225+
CHECK(scanSel.scanNames == std::list<std::string>{"Scan1"});
228226
}
229227
}
230228
}

‎src/Plugins/OrientationAnalysis/test/simpl_conversion/6_4/CreateEnsembleInfoFilter.json‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,13 @@
1111
"DataContainerName": {
1212
"Data Container Name": "DataContainer"
1313
},
14-
"Ensemble": {
15-
"CrystalStructure": [
16-
999
17-
],
18-
"PhaseType": [
19-
999
20-
],
21-
"PhaseName": [
22-
"Primary"
23-
]
24-
},
14+
"Ensemble": [
15+
{
16+
"CrystalStructure": 1,
17+
"PhaseType": 0,
18+
"PhaseName": "Primary"
19+
}
20+
],
2521
"CellEnsembleAttributeMatrixName": "CellEnsembleData",
2622
"CrystalStructuresArrayName": "TestName",
2723
"PhaseTypesArrayName": "TestName",

‎src/Plugins/OrientationAnalysis/test/simpl_conversion/6_4/ReadH5EspritDataFilter.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@
99
"Filter_Human_Label": "Read H5 Esprit Data",
1010
"Filter_Name": "ImportH5EspritData",
1111
"InputFile": "/test/path/file.h5",
12-
"SelectedScanNames": {
13-
"1": "Scan1"
14-
},
15-
"ZSpacing": 1.0,
12+
"SelectedScanNames": [
13+
"Scan1"
14+
],
15+
"ZSpacing": 2.5,
1616
"Origin": {
1717
"x": 0.0,
1818
"y": 0.0,

‎src/Plugins/OrientationAnalysis/test/simpl_conversion/6_4/ReadH5OimDataFilter.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@
99
"Filter_Human_Label": "Read H5 Oim Data",
1010
"Filter_Name": "ImportH5OimData",
1111
"InputFile": "/test/path/file.h5",
12-
"SelectedScanNames": {
13-
"1": "Scan1"
14-
},
15-
"ZSpacing": 1.0,
12+
"SelectedScanNames": [
13+
"Scan1"
14+
],
15+
"ZSpacing": 2.5,
1616
"Origin": {
1717
"x": 0.0,
1818
"y": 0.0,

‎src/Plugins/OrientationAnalysis/test/simpl_conversion/6_5/CreateEnsembleInfoFilter.json‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,13 @@
1212
"DataContainerName": {
1313
"Data Container Name": "DataContainer"
1414
},
15-
"Ensemble": {
16-
"CrystalStructure": [
17-
999
18-
],
19-
"PhaseType": [
20-
999
21-
],
22-
"PhaseName": [
23-
"Primary"
24-
]
25-
},
15+
"Ensemble": [
16+
{
17+
"CrystalStructure": 1,
18+
"PhaseType": 0,
19+
"PhaseName": "Primary"
20+
}
21+
],
2622
"CellEnsembleAttributeMatrixName": "CellEnsembleData",
2723
"CrystalStructuresArrayName": "TestName",
2824
"PhaseTypesArrayName": "TestName",

‎src/Plugins/OrientationAnalysis/test/simpl_conversion/6_5/ReadH5EspritDataFilter.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@
1010
"Filter_Name": "ImportH5EspritData",
1111
"Filter_Uuid": "{8abdea7d-f715-5a24-8165-7f946bbc2fe9}",
1212
"InputFile": "/test/path/file.h5",
13-
"SelectedScanNames": {
14-
"1": "Scan1"
15-
},
16-
"ZSpacing": 1.0,
13+
"SelectedScanNames": [
14+
"Scan1"
15+
],
16+
"ZSpacing": 2.5,
1717
"Origin": {
1818
"x": 0.0,
1919
"y": 0.0,

0 commit comments

Comments
 (0)