juli/OrcaSlicer
1
1
Fork 0
mirror of https://github.com/OrcaSlicer/OrcaSlicer.git synced 2026-10-10 17:21:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: size the destination row before migrating per-variant values

Browse Source 
update_values_from_multi_to_multi_2 iterates the destination PRINTER's variant
list while writing into a row taken from the destination PRINT preset. Those two
lengths are maintained independently -- print_extruder_variant against
printer_extruder_variant -- and Tab::load_current_preset() runs the migration
before the print preset is re-selected for the new printer. Opening a project
saved on a single-variant printer and switching to a seven-variant one therefore
wrote six elements past the end of a one-element vector. The corruption stays
silent until the next allocation, so the abort surfaces somewhere unrelated and
the backtrace points at innocent code.

Size the row to the variant count before indexing it. Every write is then in
range, and the result carries one value per destination variant, which is what
the callers consume. Pad with nil rather than a copied value: set_to_index()
skips nil entries, so a variant the object has no opinion about keeps tracking
the print preset instead of being pinned to another variant's number.

The same shape -- a count from one array indexing another -- appears twice more
in this file. update_values_from_multi_to_multi has three of these writes
protected only by assert(idx < old_count), and NDEBUG is defined for every
non-Debug configuration, so those guards are absent from shipping builds.
update_values_from_single_to_multi has the read half. Both are bounded here;
leaving them would fix one third of one defect.

Source reads are bounded too. is_nil(size_t) indexes values[idx] without
checking, so an index past the end was undefined behaviour on that side as well.

Where the row already matches the variant list -- every case that was not
corrupting the heap -- the resize is a no-op and the output is unchanged.

Fixes #15455

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clifford Garwood
2026-08-31 01:47:45 -04:00
co-authored by Claude Opus 5
parent 9abc3bcaf7
commit 855c1b51ac
2 changed files with 131 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libslic3r/PrintConfig.cpp
+50 -7
View File
@@ -10238,6 +10238,10 @@ int DynamicPrintConfig::update_values_from_single_to_multi(DynamicPrintConfig& m
for (int index = 0; index < variant_count; index++)
{
//variant_count is the variant column width, src_opt the value array;
//they disagree when the source was authored at a different width
if (index >= (int)src_opt->values.size())
break;
if (opt->values[index] > src_opt->values[index])
opt->values[index] = src_opt->values[index];
}
@@ -10255,6 +10259,8 @@ int DynamicPrintConfig::update_values_from_single_to_multi(DynamicPrintConfig& m
for (int index = 0; index < variant_count; index++)
{
if (index >= (int)src_opt->values.size())
break;
if (opt->values[index].value > src_opt->values[index].value)
opt->values[index] = src_opt->values[index];
}
@@ -10449,6 +10455,10 @@ int DynamicPrintConfig::update_values_from_multi_to_multi(DynamicPrintConfig& ne
for(auto idx : variant_indices){
assert(idx < old_count);
//the counts come from the variant columns, the arrays from the options;
//they disagree when a config was authored at a different variant width
if (idx >= old_count || new_variant_index >= (int)opt->values.size())
continue;
if (old_values[idx] < opt->values[new_variant_index])
opt->values[new_variant_index] = old_values[idx];
}
@@ -10479,6 +10489,10 @@ int DynamicPrintConfig::update_values_from_multi_to_multi(DynamicPrintConfig& ne
for(auto idx : variant_indices){
assert(idx < old_count);
//the counts come from the variant columns, the arrays from the options;
//they disagree when a config was authored at a different variant width
if (idx >= old_count || new_variant_index >= (int)opt->values.size())
continue;
if (old_values[idx] < opt->values[new_variant_index])
opt->values[new_variant_index] = old_values[idx];
}
@@ -10509,6 +10523,8 @@ int DynamicPrintConfig::update_values_from_multi_to_multi(DynamicPrintConfig& ne
for(auto idx : variant_indices){
assert(idx < old_count);
if (idx >= old_count || new_variant_index >= (int)opt->values.size())
continue;
if (old_values[idx]) //enabled
opt->values[new_variant_index] = old_values[idx];
}
@@ -10549,6 +10565,15 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vector<st
same_variant_indices.emplace_back(indices);
}
//dst_values below is the destination PRINT preset's per-variant row, sized to its own
//print_extruder_variant; dst_extruder_variants is the PRINTER's list. They disagree until
//the print preset is re-selected, so size the row to the variant count before indexing it.
const size_t dst_variant_count = dst_extruder_variants.size();
if (dst_variant_count == 0) {
BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << boost::format(", Line %1%: empty destination variant list")%__LINE__;
return -1;
}
t_config_option_keys keys = this->keys();
for(auto& key : keys){
if(key_sets.find(key) == key_sets.end())
@@ -10564,7 +10589,13 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vector<st
{
ConfigOptionFloatsNullable* opt = this->option<ConfigOptionFloatsNullable>(key);
auto src_values = opt->values;
auto dst_values = dst_config.option<ConfigOptionFloatsNullable>(key) ->values;
const auto* dst_opt = dst_config.option<ConfigOptionFloatsNullable>(key);
if(!dst_opt){
BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << boost::format(", Line %1%: %2% missing from destination config")%__LINE__%key;
break;
}
auto dst_values = dst_opt->values;
dst_values.resize(dst_variant_count, ConfigOptionFloatsNullable::nil_value());
for(size_t dst_idx =0; dst_idx < same_variant_indices.size(); ++dst_idx){
auto& indices = same_variant_indices[dst_idx];
if(indices.empty())
@@ -10572,7 +10603,7 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vector<st
bool has_value = false;
double target_value = std::numeric_limits<double>::max();
for(auto idx : indices){
if(opt && idx < opt->values.size() && !opt->is_nil(idx)){
if(idx < (int)opt->values.size() && !opt->is_nil(idx)){
has_value = true;
target_value = std::min(target_value, src_values[idx]);
}
@@ -10588,7 +10619,13 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vector<st
{
ConfigOptionFloatsOrPercentsNullable* opt = this->option<ConfigOptionFloatsOrPercentsNullable>(key);
auto src_values = opt->values;
auto dst_values = dst_config.option<ConfigOptionFloatsOrPercentsNullable>(key) ->values;
const auto* dst_opt = dst_config.option<ConfigOptionFloatsOrPercentsNullable>(key);
if(!dst_opt){
BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << boost::format(", Line %1%: %2% missing from destination config")%__LINE__%key;
break;
}
auto dst_values = dst_opt->values;
dst_values.resize(dst_variant_count, ConfigOptionFloatsOrPercentsNullable::nil_value());
for(size_t dst_idx =0; dst_idx < same_variant_indices.size(); ++dst_idx){
auto& indices = same_variant_indices[dst_idx];
if(indices.empty())
@@ -10596,7 +10633,7 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vector<st
bool has_value = false;
FloatOrPercent target_value{9999.f, true};
for(auto idx : indices){
if(opt && !opt->is_nil(idx)){
if(idx < (int)opt->values.size() && !opt->is_nil(idx)){
has_value = true;
target_value = src_values[idx].value < target_value.value ? src_values[idx] : target_value;
}
@@ -10612,15 +10649,21 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vector<st
{
ConfigOptionBoolsNullable* opt = this->option<ConfigOptionBoolsNullable>(key);
auto src_values = opt->values;
auto dst_values = dst_config.option<ConfigOptionBoolsNullable>(key) ->values;
const auto* dst_opt = dst_config.option<ConfigOptionBoolsNullable>(key);
if(!dst_opt){
BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << boost::format(", Line %1%: %2% missing from destination config")%__LINE__%key;
break;
}
auto dst_values = dst_opt->values;
dst_values.resize(dst_variant_count, ConfigOptionBoolsNullable::nil_value());
for(size_t dst_idx =0; dst_idx < same_variant_indices.size(); ++dst_idx){
auto indices = same_variant_indices[dst_idx];
if(indices.empty())
continue;
bool has_value = false;
bool target_value;
bool target_value = false;
for(auto idx : indices){
if(opt && !opt->is_nil(idx)){
if(idx < (int)opt->values.size() && !opt->is_nil(idx)){
has_value = true;
target_value = src_values[idx];
break;
tests/libslic3r/test_config_variant_expansion.cpp
+81
View File
@@ -479,3 +479,84 @@ TEST_CASE("update_values_to_printer_extruders_for_multiple_filaments resolves pe
REQUIRE(config.option<ConfigOptionInts>("filament_self_index")->values == std::vector<int>({1, 2}));
}
}
// update_values_from_multi_to_multi_2 walks the DESTINATION PRINTER's variant list while writing
// into a row taken from the destination PRINT preset, whose arrays are sized to its own
// print_extruder_variant. Those two widths disagree until the print preset is re-selected for the
// new printer -- Tab::load_current_preset() runs this migration first -- so a project authored on
// a single-variant printer, opened and switched to a wider one, wrote past the end of the row.
TEST_CASE("update_values_from_multi_to_multi_2 sizes the destination row to the variant count",
"[Config][VariantExpansion]")
{
const std::vector<std::string> src_variants{"Direct Drive Standard"};
const std::vector<std::string> dst_variants{"Direct Drive Standard", "Direct Drive High Flow",
"Direct Drive Standard", "Direct Drive High Flow"};
const std::set<std::string> keys{"outer_wall_speed"};
// The per-object override as authored on the single-variant printer.
const auto object_override = [] {
DynamicPrintConfig c;
c.option<ConfigOptionFloatsNullable>("outer_wall_speed", true)->values = {42.};
return c;
};
SECTION("a row narrower than the variant list is grown, not overrun") {
DynamicPrintConfig object_config = object_override();
DynamicPrintConfig dst;
dst.option<ConfigOptionFloatsNullable>("outer_wall_speed", true)->values = {200.};
REQUIRE(object_config.update_values_from_multi_to_multi_2(src_variants, dst_variants, dst, keys) == 0);
const auto& out = object_config.option<ConfigOptionFloatsNullable>("outer_wall_speed")->values;
REQUIRE(out.size() == dst_variants.size());
// Both "Direct Drive Standard" columns match the source variant, so they take the override.
CHECK(out[0] == Catch::Approx(42.));
CHECK(out[2] == Catch::Approx(42.));
// The High Flow columns have no matching source variant: nil, so the destination keeps
// tracking the print preset rather than being pinned to another variant's value.
CHECK(std::isnan(out[1]));
CHECK(std::isnan(out[3]));
}
// The regression guard: where the row already matches the variant list -- every case that was
// not corrupting the heap -- the resize is a no-op and the output is unchanged.
SECTION("a correctly sized row is untouched") {
DynamicPrintConfig object_config = object_override();
DynamicPrintConfig dst;
dst.option<ConfigOptionFloatsNullable>("outer_wall_speed", true)->values = {200., 500., 210., 510.};
REQUIRE(object_config.update_values_from_multi_to_multi_2(src_variants, dst_variants, dst, keys) == 0);
const auto& out = object_config.option<ConfigOptionFloatsNullable>("outer_wall_speed")->values;
REQUIRE(out.size() == 4);
CHECK(out[0] == Catch::Approx(42.)); // matched -> override
CHECK(out[1] == Catch::Approx(500.)); // unmatched -> preset value preserved
CHECK(out[2] == Catch::Approx(42.));
CHECK(out[3] == Catch::Approx(510.));
}
// is_nil(idx) indexes values[idx] with no bounds check, so a source shorter than its own
// variant list read out of range before the guard was added.
SECTION("a source shorter than its variant list is read in range") {
DynamicPrintConfig object_config = object_override(); // one value...
DynamicPrintConfig dst;
dst.option<ConfigOptionFloatsNullable>("outer_wall_speed", true)->values = {200., 500.};
REQUIRE(object_config.update_values_from_multi_to_multi_2(
{"Direct Drive Standard", "Direct Drive Standard"}, // ...but two source variants
{"Direct Drive Standard", "Direct Drive High Flow"}, dst, keys) == 0);
const auto& out = object_config.option<ConfigOptionFloatsNullable>("outer_wall_speed")->values;
REQUIRE(out.size() == 2);
CHECK(out[0] == Catch::Approx(42.));
CHECK(out[1] == Catch::Approx(500.));
}
SECTION("an empty destination variant list is refused") {
DynamicPrintConfig object_config = object_override();
DynamicPrintConfig dst;
dst.option<ConfigOptionFloatsNullable>("outer_wall_speed", true)->values = {200.};
CHECK(object_config.update_values_from_multi_to_multi_2(src_variants, {}, dst, keys) == -1);
}
}