diff --git a/src/codegen/chapel.rs b/src/codegen/chapel.rs index 372a505..83b376b 100644 --- a/src/codegen/chapel.rs +++ b/src/codegen/chapel.rs @@ -797,17 +797,30 @@ fn write_gather_merge(src: &mut String) -> Result<()> { fn write_gather_reduce(src: &mut String) -> Result<()> { writeln!( src, - " // Reduce: fold all results into a single accumulator using c_reduce." + " // Reduce: fold all SUCCESSFUL results into a single accumulator." )?; writeln!( src, - " var accumBuf: [0..#maxItemBytes] uint(8) = resultData[0];" + " // Seed from the first successful item (never blindly item 0 — it may" )?; - writeln!(src, " var accumLen: c_size_t = resultSizes[0];")?; + writeln!( + src, + " // have failed), and only mark the reduced result valid if at least" + )?; + writeln!(src, " // one input succeeded and no fold step failed.")?; + writeln!(src, " var accumBuf: [0..#maxItemBytes] uint(8);")?; + writeln!(src, " var accumLen: c_size_t = 0;")?; + writeln!(src, " var haveAccum: bool = false;")?; writeln!(src)?; writeln!(src, " on Locales[0] {{")?; - writeln!(src, " for i in 1..#(nItems - 1) {{")?; + writeln!(src, " for i in 0..#nItems {{")?; writeln!(src, " if !resultOk[i] then continue;")?; + writeln!(src, " if !haveAccum {{")?; + writeln!(src, " accumBuf = resultData[i];")?; + writeln!(src, " accumLen = resultSizes[i];")?; + writeln!(src, " haveAccum = true;")?; + writeln!(src, " continue;")?; + writeln!(src, " }}")?; writeln!(src, " var tmpBuf: [0..#maxItemBytes] uint(8);")?; writeln!(src, " var tmpLen: c_size_t = 0;")?; writeln!(src, " const rc = c_reduce(")?; @@ -818,17 +831,41 @@ fn write_gather_reduce(src: &mut String) -> Result<()> { writeln!(src, " if rc == 0 {{")?; writeln!(src, " accumBuf = tmpBuf;")?; writeln!(src, " accumLen = tmpLen;")?; + writeln!(src, " }} else {{")?; + writeln!( + src, + " // A fold step failed: do not silently report a partial reduction." + )?; + writeln!( + src, + " writeln(\" ERROR: c_reduce failed at item \", i, \" (rc=\", rc, \")\");" + )?; + writeln!(src, " haveAccum = false;")?; + writeln!(src, " break;")?; writeln!(src, " }}")?; writeln!(src, " }}")?; writeln!(src, " }}")?; writeln!(src)?; - writeln!(src, " // Store reduced result as item 0")?; + writeln!( + src, + " // Store reduced result as item 0; valid only if we folded" + )?; + writeln!( + src, + " // at least one successful input with no failed step." + )?; writeln!(src, " resultData[0] = accumBuf;")?; writeln!(src, " resultSizes[0] = accumLen;")?; - writeln!(src, " resultOk[0] = true;")?; + writeln!(src, " resultOk[0] = haveAccum;")?; + writeln!(src, " if haveAccum then")?; + writeln!( + src, + " writeln(\" Reduced \", nItems, \" results into 1\");" + )?; + writeln!(src, " else")?; writeln!( src, - " writeln(\" Reduced \", nItems, \" results into 1\");" + " writeln(\" WARN: reduce produced no valid result (no successful inputs)\");" )?; Ok(()) @@ -1033,6 +1070,11 @@ fn write_store_phase(src: &mut String, manifest: &Manifest) -> Result<()> { " // ================================================================" )?; writeln!(src)?; + // A store failure means the user's result never reached their storage. Track + // it and abort with a non-zero status at the end rather than warn-and-exit-0 + // (soundness: do not report overall success when results were not stored). + writeln!(src, " var storeFailed: bool = false;")?; + writeln!(src)?; match manifest.workload.gather.as_str() { // Reduce strategies produce a single result in slot 0 @@ -1043,10 +1085,13 @@ fn write_store_phase(src: &mut String, manifest: &Manifest) -> Result<()> { src, " const rc = c_store_result(0: c_int, c_ptrTo(resultData[0][0]), resultSizes[0]);" )?; + writeln!(src, " if rc != 0 {{")?; writeln!( src, - " if rc != 0 then writeln(\" WARN: c_store_result(0) returned \", rc);" + " writeln(\" ERROR: c_store_result(0) returned \", rc);" )?; + writeln!(src, " storeFailed = true;")?; + writeln!(src, " }}")?; writeln!(src, " }}")?; writeln!(src, " }}")?; } @@ -1059,10 +1104,13 @@ fn write_store_phase(src: &mut String, manifest: &Manifest) -> Result<()> { src, " const rc = c_store_result(0: c_int, c_ptrTo(resultData[foundIdx][0]), resultSizes[foundIdx]);" )?; + writeln!(src, " if rc != 0 {{")?; writeln!( src, - " if rc != 0 then writeln(\" WARN: c_store_result(0) returned \", rc);" + " writeln(\" ERROR: c_store_result(0) returned \", rc);" )?; + writeln!(src, " storeFailed = true;")?; + writeln!(src, " }}")?; writeln!(src, " }}")?; writeln!(src, " }}")?; } @@ -1076,15 +1124,30 @@ fn write_store_phase(src: &mut String, manifest: &Manifest) -> Result<()> { src, " const rc = c_store_result(i: c_int, c_ptrTo(resultData[i][0]), resultSizes[i]);" )?; + writeln!(src, " if rc != 0 {{")?; writeln!( src, - " if rc != 0 then writeln(\" WARN: c_store_result(\", i, \") returned \", rc);" + " writeln(\" ERROR: c_store_result(\", i, \") returned \", rc);" )?; + writeln!(src, " storeFailed = true;")?; + writeln!(src, " }}")?; writeln!(src, " }}")?; writeln!(src, " }}")?; } } + writeln!(src)?; + writeln!( + src, + " // Fail loudly if any result could not be stored — never exit 0 on a" + )?; + writeln!(src, " // silent storage failure.")?; + writeln!(src, " if storeFailed then")?; + writeln!( + src, + " halt(\"ERROR: one or more results failed to store; aborting with failure status\");" + )?; + writeln!(src)?; Ok(()) } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index e916642..6d617c5 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1612,3 +1612,61 @@ fn test_regression_chapel_comm_layer_in_build() { "Build script should set CHPL_COMM from manifest" ); } + +// =========================================================================== +// Soundness: gather/store must never report success on failed inputs/stores +// =========================================================================== + +/// The reduce gather must seed its accumulator from the first SUCCESSFUL item +/// (never blindly item 0, which may have failed) and mark the reduced result +/// valid only if at least one input succeeded — `resultOk[0] = haveAccum`, +/// never an unconditional `resultOk[0] = true`. +#[test] +fn reduce_gather_is_failure_aware() { + let m = parse_manifest(&make_manifest_toml("chunk", "reduce")); + let (dir, safe_name) = generate_to_tempdir(&m); + let chpl = fs::read_to_string( + dir.path() + .join(format!("chapel/{safe_name}_distributed.chpl")), + ) + .unwrap(); + + assert!( + chpl.contains("haveAccum"), + "reduce gather must track whether any successful input was folded" + ); + assert!( + chpl.contains("resultOk[0] = haveAccum;"), + "reduce result validity must follow haveAccum" + ); + assert!( + !chpl.contains("resultOk[0] = true;"), + "reduce must not unconditionally mark the result valid" + ); +} + +/// The store phase must not silently warn-and-exit-0 on a c_store_result +/// failure: it tracks the failure and halts with a non-zero status. +#[test] +fn store_phase_aborts_on_store_failure() { + let m = parse_manifest(&make_manifest_toml("chunk", "merge")); + let (dir, safe_name) = generate_to_tempdir(&m); + let chpl = fs::read_to_string( + dir.path() + .join(format!("chapel/{safe_name}_distributed.chpl")), + ) + .unwrap(); + + assert!( + chpl.contains("storeFailed"), + "store phase must track store failures" + ); + assert!( + chpl.contains("halt("), + "store phase must halt (non-zero exit) when a result fails to store" + ); + assert!( + !chpl.contains("WARN: c_store_result"), + "store failures must be errors, not silent warnings" + ); +}