juli/OrcaSlicer
1
1
Fork 0
mirror of https://github.com/OrcaSlicer/OrcaSlicer.git synced 2026-09-01 22:37:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: out-of-bounds write migrating per-variant values when switching printers (#15456)

Browse Source 
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clifford
2026-09-01 18:29:11 -04:00
committed by GitHub
parent e8115658e0
commit b6ef6cf1be
2 changed files with 131 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
src/libslic3r/PrintConfig.cpp
View File

@@ -10091,6 +10091,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];
}
@@ -10108,6 +10112,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];
}
@@ -10302,6 +10308,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];
}
@@ -10332,6 +10342,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];
}
@@ -10362,6 +10376,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];
}
@@ -10402,6 +10418,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())
@@ -10417,7 +10442,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())
@@ -10425,7 +10456,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]);
}
@@ -10441,7 +10472,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())
@@ -10449,7 +10486,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;
}
@@ -10465,15 +10502,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;

81
tests/libslic3r/test_config_variant_expansion.cpp
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);
}
}