Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/analyses/apron/relationAnalysis.apron.ml
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ struct
Priv.write_global ask getg sideg st g x
else (
let rel = st.rel in
let g_var = RV.global g in
(* Escaped locals remain represented by their local variable until the
program becomes multithreaded, just like in [read_global]. *)
let g_var = if g.vglob then RV.global g else RV.local g in
let x_var = RV.local x in
let rel' = RD.add_vars rel [g_var] in
let rel' = RD.assign_var rel' g_var x_var in
Expand Down Expand Up @@ -327,7 +329,7 @@ struct
let any_local_reachable = any_local_reachable fundec reachable_from_args in
RD.remove_filter_with new_rel (fun var ->
match RV.find_metadata var with
| Some (Local _) when not (belongs_to_fundec fundec var || any_local_reachable) -> true (* remove caller locals provided they are unreachable *)
| Some (Local v) when not (belongs_to_fundec fundec var || any_local_reachable || ThreadEscape.has_escaped (Analyses.ask_of_man man) v) -> true (* remove caller locals provided they are unreachable *)
| Some (Arg _) when not (List.mem_cmp Apron.Var.compare var arg_vars) -> true (* remove caller args, but keep just added args *)
| _ -> false (* keep everything else (just added args, globals, global privs) *)
);
Expand Down Expand Up @@ -422,6 +424,7 @@ struct
let tainted_vars = TaintPartialContexts.conv_varset tainted in
let new_rel = RD.keep_filter st.rel (fun var ->
match RV.find_metadata var with
| Some (Local v) when ThreadEscape.has_escaped ask v -> false (* escaped locals may be modified through globals *)
| Some (Local _) when not (belongs_to_fundec fundec var || any_local_reachable) -> true (* keep caller locals, provided they were not passed to the function *)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How come this is different from the make_callee_rel change above?
Previously, both used the exact same condition (negated) to either remove or keep the corresponding locals.
Now, they are no longer symmetric like that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is not semantically different here, it's just that I broke the symmetry in formatting. I have restored the symmetry now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Initially I thought so too that it's just different formatting, but that's not true.
For an escaped local:

  • Before the symmetry fix, this immediately goes to false.
  • After the symmetry fix, all the negated escape condition does is not allow it to go to that true. But there are other Local cases below with different conditions that may still return true.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's true, I hadn't thought about this. Looking at it, the case where this can now actually go to true is

| Some ((Local v | Global v)) when not (TaintPartialContexts.VS.mem v tainted_vars) -> true (* keep locals and globals, which have not been touched by the call *)

This is correct: If the escaped variable was definitely not modified in the callee, we can preserve the caller state.

| Some (Arg _) -> true (* keep caller args *)
| Some ((Local _ | Global _)) when not (RD.mem_var new_fun_rel var) -> false (* remove locals and globals, for which no record exists in the new_fun_apr *)
Expand Down
17 changes: 17 additions & 0 deletions tests/regression/89-apron3/05-escaped-local.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// PARAM: --set ana.activated[+] apron --enable ana.sv-comp.functions
#include <goblint.h>

static int *target;

static void modify(void) {
*target = __VERIFIER_nondet_int();
}

int main(void) {
int left = __VERIFIER_nondet_int();
int right = left;
target = &right;

modify();
__goblint_check(left == right); // UNKNOWN!
}
25 changes: 25 additions & 0 deletions tests/regression/89-apron3/06-escaped-local-enter-multithreaded.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// PARAM: --set ana.activated[+] apron --enable ana.sv-comp.functions
#include <goblint.h>
#include <pthread.h>

static int *target;

static void *modify(void *arg) {
*target = __VERIFIER_nondet_int();
return NULL;
}

static void enter_multithreaded_and_modify(void) {
pthread_t thread;
pthread_create(&thread, NULL, modify, NULL);
pthread_join(thread, NULL);
}

int main(void) {
int left = __VERIFIER_nondet_int();
int right = left;
target = &right;

enter_multithreaded_and_modify();
__goblint_check(left == right); // UNKNOWN!
}
Loading