Skip to content

Conversation

@michael-schwarz
Copy link
Member

@michael-schwarz michael-schwarz commented Aug 1, 2025

This generates an equivalent forward propagating constraint system:

  • Uses ForwardCFG with all updates happening via sidel
  • Instantiates globals as spec globals plus with auxiliary globals for returns per fundec and context
  • In tf_proc use getg to get end state of procedure
  • In tf_ret side effect to appropriate helper global

It also presents Helmut's new forward solver.

@DrMichaelPetter DrMichaelPetter self-requested a review August 3, 2025 05:48
@michael-schwarz
Copy link
Member Author

michael-schwarz commented Aug 5, 2025

This is now a rudimentary version of the solver that works. Missing for something we can use in Goblint are:

  • Post-solver like iteration over results to produce warnings (using check) (done)
  • Fix detection of uncalled functions (done)
  • Extensive testing
  • Refactoring to increase reuse between constraints.ml and fwdConstraints.ml
  • Refactoring to avoid duplication between control.ml and fwdControl.ml

Plus all further features such as witness generation, SV-COMP verdicts, etc

@michael-schwarz
Copy link
Member Author

Also, we need a new concept of dead code detection, it is no longer the case that we will explicitly compute bot for such nodes.

Comment on lines +174 to +204
let () = LM.add last x d in
let set = LS.add x set in
let d_new = get_last_contrib sx set last in
let (new_xg,delay,gas,narrow) = gwarrow (old_xg,delay,gas,narrow) d_new in
let () = OM.replace from sx (new_xg,delay,gas,narrow,set) in
if G.equal new_xg old_xg then (
if tracing then trace "set_globalc" "no change!";
)
else
begin
if tracing then trace "set_globalc" "new contribution registered!";
let new_g = get_global_value init from in
if G.equal value new_g then
()
else
let () = GM.replace glob g {value = new_g; init = init; infl = LS.empty;last;from} in
let work = infl in
let _ = GM.replace glob g {value = new_g; init = init; infl = LS.empty; last; from} in
let doit x =
let r = get_local_ref x in
if !(r.called) then r.aborted := true
else (
if tracing then trace "iter" "set_global caused iter\n By: %a\nLocal:%a" System.GVar.pretty_trace g System.LVar.pretty_trace x;
iterate x
)
in
(*
if tracing then trace "iter" "Size of work: %d" (LS.seq_of work |> Seq.length);
*)
LS.iter doit work
end

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: semgrep.let-unit-in Warning

use ; instead (and, if needed, add surrounding parentheses to preserve precedence)
Comment on lines +178 to +204
let () = OM.replace from sx (new_xg,delay,gas,narrow,set) in
if G.equal new_xg old_xg then (
if tracing then trace "set_globalc" "no change!";
)
else
begin
if tracing then trace "set_globalc" "new contribution registered!";
let new_g = get_global_value init from in
if G.equal value new_g then
()
else
let () = GM.replace glob g {value = new_g; init = init; infl = LS.empty;last;from} in
let work = infl in
let _ = GM.replace glob g {value = new_g; init = init; infl = LS.empty; last; from} in
let doit x =
let r = get_local_ref x in
if !(r.called) then r.aborted := true
else (
if tracing then trace "iter" "set_global caused iter\n By: %a\nLocal:%a" System.GVar.pretty_trace g System.LVar.pretty_trace x;
iterate x
)
in
(*
if tracing then trace "iter" "Size of work: %d" (LS.seq_of work |> Seq.length);
*)
LS.iter doit work
end

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: semgrep.let-unit-in Warning

use ; instead (and, if needed, add surrounding parentheses to preserve precedence)
Comment on lines +189 to +203
let () = GM.replace glob g {value = new_g; init = init; infl = LS.empty;last;from} in
let work = infl in
let _ = GM.replace glob g {value = new_g; init = init; infl = LS.empty; last; from} in
let doit x =
let r = get_local_ref x in
if !(r.called) then r.aborted := true
else (
if tracing then trace "iter" "set_global caused iter\n By: %a\nLocal:%a" System.GVar.pretty_trace g System.LVar.pretty_trace x;
iterate x
)
in
(*
if tracing then trace "iter" "Size of work: %d" (LS.seq_of work |> Seq.length);
*)
LS.iter doit work

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: semgrep.let-unit-in Warning

use ; instead (and, if needed, add surrounding parentheses to preserve precedence)
Comment on lines +198 to +227
let () = LM.add last x d in
let set = LS.add x set in
let d_new = get_last_contrib sx set last in
let (new_xg,delay,gas,narrow) = gwarrow (old_xg,delay,gas,narrow) d_new in
let () = OM.replace from sx (new_xg,delay,gas,narrow,set) in
if G.equal new_xg old_xg then (
if tracing then trace "set_globalc" "no change!"
)
else
begin
if tracing then trace "set_globalc" "new contribution registered!";
let new_g = get_global_value init from in
if G.equal value new_g then
()
else
let work = infl in
let _ = GM.replace glob g {value = new_g; init = init; infl = LS.empty; last; from} in
let doit x =
let r = get_local_ref x in
if !(r.called) then r.aborted := true
else (
if tracing then trace "iter" "set_global caused iter\n By: %a\nLocal:%a" System.GVar.pretty_trace g System.LVar.pretty_trace x;
add_set x
)
in
(*
if tracing then trace "iter" "Size of work: %d" (LS.seq_of work |> Seq.length);
*)
LS.iter doit work
end

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: semgrep.let-unit-in Warning

use ; instead (and, if needed, add surrounding parentheses to preserve precedence)
Comment on lines +202 to +227
let () = OM.replace from sx (new_xg,delay,gas,narrow,set) in
if G.equal new_xg old_xg then (
if tracing then trace "set_globalc" "no change!"
)
else
begin
if tracing then trace "set_globalc" "new contribution registered!";
let new_g = get_global_value init from in
if G.equal value new_g then
()
else
let work = infl in
let _ = GM.replace glob g {value = new_g; init = init; infl = LS.empty; last; from} in
let doit x =
let r = get_local_ref x in
if !(r.called) then r.aborted := true
else (
if tracing then trace "iter" "set_global caused iter\n By: %a\nLocal:%a" System.GVar.pretty_trace g System.LVar.pretty_trace x;
add_set x
)
in
(*
if tracing then trace "iter" "Size of work: %d" (LS.seq_of work |> Seq.length);
*)
LS.iter doit work
end

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: semgrep.let-unit-in Warning

use ; instead (and, if needed, add surrounding parentheses to preserve precedence)
… collection of unknowns representing the same origin
Comment on lines +156 to +157
let () = OM.add from sx tuple in
tuple in

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: semgrep.let-unit-in Warning

use ; instead (and, if needed, add surrounding parentheses to preserve precedence)
Comment on lines +158 to +173
let () = HM.add last x d in
let set = VS.add x set in
let d = VS.fold (fun x d -> S.Dom.join d (HM.find last x)) set d in
let (new_xy,delay,gas,narrow) =
if M.tracing then M.trace "wpoint" "side widen %a" S.Var.pretty_trace y;
warrow (old_xy,delay,gas,narrow) d in
OM.replace from sx (new_xy,delay,gas,narrow,set);
if S.Dom.equal new_xy old_xy then ()
else (
let new_y = get_global_value init from in
if S.Dom.equal new_y !y_ref.value then ()
else (
y_ref := { !y_ref with value = new_y };
destabilize y
)
)

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: semgrep.let-unit-in Warning

use ; instead (and, if needed, add surrounding parentheses to preserve precedence)
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a forward propagating constraint system for Goblint's static analysis framework. The new system uses ForwardCFG with side effects via sidel operations and introduces three new solver implementations (forward, bottom-up, and widening bottom-up) along with Helmut's improved top-down solver. The PR integrates these solvers into the main analysis pipeline with conditional activation based on solver configuration.

Key Changes

  • Added three new forward-propagating solvers (FwdSolver, BuSolver, WBuSolver) with corresponding constraint and control modules
  • Introduced Helmut's improved top-down solver with enhanced origin tracking for narrowing on globals
  • Integrated solver selection logic to conditionally use forward control flow when "fwd", "wbu", or "bu" solvers are specified

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test.c Test case for validating the forward constraint system with interval analysis
src/solver/td_simplified_ref_improved.ml New top-down solver with record-based state management and origin tracking for narrowing
src/maingoblint.ml Added conditional solver selection logic and fixed indentation in gobview code block
src/goblint_lib.ml Exported new FwdControl, FwdSolver, and FwdConstraints modules
src/goblint.ml Fixed spelling of "independent" in comment
src/framework/wbu.ml Widening bottom-up solver implementation with work set management
src/framework/fwdSolver.ml Main forward solver with local/global variable handling and warrowing
src/framework/fwdControl.ml Forward control flow analysis module with solver integration
src/framework/fwdConstraints.ml Forward constraint generation from analysis specifications
src/framework/constraints.ml Fixed spelling of "Transformations" in comment
src/framework/bu.ml Bottom-up solver implementation with iterative fixpoint computation
src/framework/analyses.ml Added GVarFCNW and GVarL module types for forward constraint system
src/constraint/constrSys.ml Added FwdGlobConstrSys module type interface
lib/goblint/runtime/src/dune Added C11 standard flag as workaround for issue #1779

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

let solve = if (get_string "solver" = "bu") then BuSolver.solve else
if (get_string "solver" = "wbu") then WBuSolver.solve else FwdSlvr.solve in
let check = if (get_string "solver" = "bu") then BuSolver.check else
if (get_string "solver" = "wbu") then WBuSolver.solve else FwdSlvr.check in
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check function is incorrectly assigned. Line 480 assigns WBuSolver.solve to the check variable, but it should assign WBuSolver.check to match the pattern on line 479.

Suggested change
if (get_string "solver" = "wbu") then WBuSolver.solve else FwdSlvr.check in
if (get_string "solver" = "wbu") then WBuSolver.check else FwdSlvr.check in

Copilot uses AI. Check for mistakes.
else
let () = GM.replace glob g {value = new_g; init = init; infl = LS.empty;last;from} in
let work = infl in
let _ = GM.replace glob g {value = new_g; init = init; infl = LS.empty; last; from} in
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is duplicate code on lines 189 and 191 that replaces the same global variable with identical values. The first replacement on line 189 is redundant since it's immediately overwritten by line 191.

Suggested change
let _ = GM.replace glob g {value = new_g; init = init; infl = LS.empty; last; from} in

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
michael-schwarz added a commit that referenced this pull request Dec 17, 2025
michael-schwarz and others added 2 commits December 17, 2025 18:59
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants