Skip to content

Commit a4f1655

Browse files
committed
Fix three-way-merge to be field-sensitive with --vsd-structs every-field
For structs with struct-typed fields, merging now recursively descends to identify leaf-level modifications. Array handling uses a conservative approach (marking entire array as modified) since enumerating all indices is not practical. Co-authored-by: Kiro autonomous agent Fixes: #6272
1 parent 76dd20f commit a4f1655

File tree

3 files changed

+171
-11
lines changed

3 files changed

+171
-11
lines changed
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
#include <assert.h>
2+
3+
struct s
4+
{
5+
int x;
6+
int y;
7+
};
8+
9+
struct s global;
10+
int alsoGlobal;
11+
12+
void f00(void)
13+
{
14+
global.x = 0;
15+
}
16+
17+
int main(int argc, char **argv)
18+
{
19+
global.x = 1;
20+
global.y = 1;
21+
alsoGlobal = 1;
22+
23+
f00();
24+
25+
assert(global.x == 0);
26+
assert(global.y == 1);
27+
assert(alsoGlobal == 1);
28+
29+
global.x = 2;
30+
global.y = 2;
31+
alsoGlobal = 2;
32+
33+
f00();
34+
35+
assert(global.x == 0);
36+
assert(global.y == 2);
37+
assert(alsoGlobal == 2);
38+
39+
return 0;
40+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
CORE
2+
main.c
3+
--verify --vsd --three-way-merge --vsd-structs every-field
4+
^\[main.assertion.1\] line \d+ assertion global.x == 0: SUCCESS$
5+
^\[main.assertion.2\] line \d+ assertion global.y == 1: SUCCESS$
6+
^\[main.assertion.3\] line \d+ assertion alsoGlobal == 1: SUCCESS$
7+
^\[main.assertion.4\] line \d+ assertion global.x == 0: SUCCESS$
8+
^\[main.assertion.5\] line \d+ assertion global.y == 2: SUCCESS$
9+
^\[main.assertion.6\] line \d+ assertion alsoGlobal == 2: SUCCESS$
10+
^EXIT=0$
11+
^SIGNAL=0$
12+
--

src/analyses/variable-sensitivity/variable_sensitivity_domain.cpp

Lines changed: 119 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Date: April 2016
1313
#include <util/cprover_prefix.h>
1414
#include <util/namespace.h>
1515
#include <util/pointer_expr.h>
16+
#include <util/std_expr.h>
1617
#include <util/symbol_table_base.h>
1718

1819
#include <algorithm>
@@ -416,6 +417,90 @@ bool variable_sensitivity_domaint::ignore_function_call_transform(
416417
ignored_internal_function.cend();
417418
}
418419

420+
// Helper function to recursively collect all modified sub-paths within an
421+
// object
422+
static void collect_modified_paths(
423+
const abstract_object_pointert &start_value,
424+
const abstract_object_pointert &end_value,
425+
const exprt &base_expr,
426+
std::vector<exprt> &modified_paths,
427+
const abstract_environmentt &env,
428+
const namespacet &ns)
429+
{
430+
// If the objects themselves are different at this level
431+
if(start_value->has_been_modified(end_value))
432+
{
433+
// Check if this is a struct or array that we should descend into
434+
const typet &obj_type = start_value->type();
435+
436+
if(obj_type.id() == ID_struct || obj_type.id() == ID_struct_tag)
437+
{
438+
// Get the struct type
439+
const struct_typet &struct_type =
440+
obj_type.id() == ID_struct
441+
? to_struct_type(obj_type)
442+
: ns.follow_tag(to_struct_tag_type(obj_type));
443+
444+
// Track if any field was modified
445+
bool any_field_modified = false;
446+
447+
// Check each field
448+
for(const auto &component : struct_type.components())
449+
{
450+
member_exprt member_expr(
451+
base_expr, component.get_name(), component.type());
452+
453+
abstract_object_pointert start_field =
454+
start_value->expression_transform(
455+
member_expr, {start_value}, env, ns);
456+
abstract_object_pointert end_field =
457+
end_value->expression_transform(member_expr, {end_value}, env, ns);
458+
459+
// Recursively check if this field was modified
460+
if(start_field->has_been_modified(end_field))
461+
{
462+
any_field_modified = true;
463+
464+
// Check if this field is also a struct/array - if so, descend
465+
// recursively
466+
const typet &field_type = start_field->type();
467+
if(
468+
field_type.id() == ID_struct || field_type.id() == ID_struct_tag ||
469+
field_type.id() == ID_array)
470+
{
471+
// Recursively descend
472+
collect_modified_paths(
473+
start_field, end_field, member_expr, modified_paths, env, ns);
474+
}
475+
else
476+
{
477+
// It's a leaf field (not struct/array), add it
478+
modified_paths.push_back(member_expr);
479+
}
480+
}
481+
}
482+
483+
// If no field-specific modifications were found but the struct was marked
484+
// as modified, fall back to updating the whole struct
485+
if(!any_field_modified)
486+
{
487+
modified_paths.push_back(base_expr);
488+
}
489+
}
490+
else if(obj_type.id() == ID_array)
491+
{
492+
// For arrays, we can't enumerate all possible indices
493+
// So we fall back to marking the entire array as modified
494+
modified_paths.push_back(base_expr);
495+
}
496+
else
497+
{
498+
// It's a simple value type that was modified
499+
modified_paths.push_back(base_expr);
500+
}
501+
}
502+
}
503+
419504
void variable_sensitivity_domaint::merge_three_way_function_return(
420505
const ai_domain_baset &function_start,
421506
const ai_domain_baset &function_end,
@@ -431,19 +516,42 @@ void variable_sensitivity_domaint::merge_three_way_function_return(
431516
abstract_environmentt::modified_symbols(
432517
cast_function_start.abstract_state, cast_function_end.abstract_state);
433518

434-
std::vector<symbol_exprt> modified_symbols;
435-
modified_symbols.reserve(modified_symbol_names.size());
436-
std::transform(
437-
modified_symbol_names.begin(),
438-
modified_symbol_names.end(),
439-
std::back_inserter(modified_symbols),
440-
[&ns](const irep_idt &id) { return ns.lookup(id).symbol_expr(); });
441-
442-
for(const auto &symbol : modified_symbols)
519+
for(const auto &symbol_name : modified_symbol_names)
443520
{
444-
abstract_object_pointert value =
521+
symbol_exprt symbol = ns.lookup(symbol_name).symbol_expr();
522+
523+
abstract_object_pointert function_start_value =
524+
cast_function_start.abstract_state.eval(symbol, ns);
525+
abstract_object_pointert function_end_value =
445526
cast_function_end.abstract_state.eval(symbol, ns);
446-
abstract_state.assign(symbol, value, ns);
527+
528+
// Collect all modified sub-paths (fields/elements) within this symbol
529+
std::vector<exprt> modified_paths;
530+
collect_modified_paths(
531+
function_start_value,
532+
function_end_value,
533+
symbol,
534+
modified_paths,
535+
cast_function_start.abstract_state,
536+
ns);
537+
538+
// Apply updates only for the modified paths
539+
if(modified_paths.empty())
540+
{
541+
// No specific sub-paths found, update the whole symbol
542+
// This happens for non-struct/non-array types
543+
abstract_state.assign(symbol, function_end_value, ns);
544+
}
545+
else
546+
{
547+
// Update only the modified sub-paths
548+
for(const auto &path : modified_paths)
549+
{
550+
abstract_object_pointert value =
551+
cast_function_end.abstract_state.eval(path, ns);
552+
abstract_state.assign(path, value, ns);
553+
}
554+
}
447555
}
448556

449557
return;

0 commit comments

Comments
 (0)