mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-08-08 10:37:40 +00:00
Fix out-of-bounds reads in the multi-extruder flush-volume matrix
flush_volumes_matrix stores one (filaments x filaments) block per nozzle, but set_extruder, WipeTower2::extract_wipe_volumes and the PresetBundle rebuild indexed each block with filament_colour.size() as the row stride. When the stored matrix does not match that assumption (a legacy project, or a just-switched printer -- e.g. a single 2x2 block while nozzle_diameter has 2 entries) the index runs past the sliced block and reads out of bounds. The read is undefined, so its value depends on the C++ std-lib/allocator layout: identical across Linux architectures but different on macOS/Windows. That surfaced as the "Toolchange temperature commands are unchanged when the wipe tower wait is off" regression failing on Linux CI while passing on macOS/Windows, and crashing hardened (-O0, _GLIBCXX_ASSERTIONS) builds. - Centralize the block-dimension derivation in get_flush_volumes_matrix_dims (sqrt(size / nozzles) with a filaments^2 * nozzles == size check and a single-block fallback) and use it wherever the matrix is sliced/indexed; bounds-guard the reads as defense in depth. It weighs both options that claim to say how many blocks are stored, since either can be stale: flush_multiplier, written with the matrix in the project config, and nozzle_diameter, which changes the moment a printer is selected. - PresetBundle::update_multi_material_filament_presets: rebuild the matrix on a nozzle-count-only change too (the old per-block gate missed those), and seed a brand-new nozzle from the first nozzle's tuned block. - is_flush_config_modified: stride the stored matrix by its own dimension rather than the current filament count, and bound the nozzle loop by the printer's extruder count, which CalcFlushingVolumes indexes as well. - is_flushing_matrix_error: the same derivation, which additionally divided by zero on an empty flush_multiplier. - update_slice_warnings: guard nozzle_hrc_lists, which is sized by the nullable nozzle_type option and can be shorter than the extruder count. With the read fixed the emitted trace is deterministic across builds and platforms, so regenerate the golden from it and stop comparing the rounded "time: <n>s" preheat comment (the tolerant lead time already carries that timing). Add unit tests for the flush-matrix rebuild and dimension logic.
This commit is contained in:
@@ -175,6 +175,17 @@ static std::pair<std::string, std::optional<double>> split_lead(const std::strin
|
||||
return { entry.substr(0, tab), std::stod(tail.substr(tail.find(' ') + 1)) };
|
||||
}
|
||||
|
||||
// The "time: <n>s" a preheat comment carries is round()'d from the same estimate the lead measures,
|
||||
// so it sits on a rounding boundary and flips (e.g. 30<->31) across platforms and build optimisation
|
||||
// levels; drop it from the command text so only the tolerant lead below carries that timing.
|
||||
static std::string strip_preheat_time(std::string command)
|
||||
{
|
||||
const size_t pos = command.find(" time: ");
|
||||
if (pos != std::string::npos)
|
||||
command.erase(pos);
|
||||
return command;
|
||||
}
|
||||
|
||||
// Same command, and a lead time within half a second. The lead is an estimate summed over every
|
||||
// move before it, so it drifts slightly with unrelated changes to travel or tower geometry; half a
|
||||
// second is far below the tens of seconds a preheat leaving its backtrace position would shift it.
|
||||
@@ -182,7 +193,7 @@ static bool trace_entries_match(const std::string& a, const std::string& b)
|
||||
{
|
||||
const auto x = split_lead(a);
|
||||
const auto y = split_lead(b);
|
||||
if (x.first != y.first)
|
||||
if (strip_preheat_time(x.first) != strip_preheat_time(y.first))
|
||||
return false;
|
||||
if (x.second.has_value() != y.second.has_value())
|
||||
return false;
|
||||
|
||||
Reference in New Issue
Block a user