Skip to content

Commit 459076c

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 459076c

File tree

3 files changed

+169
-11
lines changed

3 files changed

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

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)
517+
for(const auto &symbol_name : modified_symbol_names)
443518
{
444-
abstract_object_pointert value =
519+
symbol_exprt symbol = ns.lookup(symbol_name).symbol_expr();
520+
521+
abstract_object_pointert function_start_value =
522+
cast_function_start.abstract_state.eval(symbol, ns);
523+
abstract_object_pointert function_end_value =
445524
cast_function_end.abstract_state.eval(symbol, ns);
446-
abstract_state.assign(symbol, value, ns);
525+
526+
// Collect all modified sub-paths (fields/elements) within this symbol
527+
std::vector<exprt> modified_paths;
528+
collect_modified_paths(
529+
function_start_value,
530+
function_end_value,
531+
symbol,
532+
modified_paths,
533+
cast_function_start.abstract_state,
534+
ns);
535+
536+
// Apply updates only for the modified paths
537+
if(modified_paths.empty())
538+
{
539+
// No specific sub-paths found, update the whole symbol
540+
// This happens for non-struct/non-array types
541+
abstract_state.assign(symbol, function_end_value, ns);
542+
}
543+
else
544+
{
545+
// Update only the modified sub-paths
546+
for(const auto &path : modified_paths)
547+
{
548+
abstract_object_pointert value =
549+
cast_function_end.abstract_state.eval(path, ns);
550+
abstract_state.assign(path, value, ns);
551+
}
552+
}
447553
}
448554

449555
return;

0 commit comments

Comments
 (0)