[pre-analysis] Remove throw/catch handling from preanalysis

Summary:
D20769039 (cec8cbeff2) added a preanalysis step that creates edges from throw nodes to all reachable catch nodes. It intended to fix some deadstore FPs however it caused more damage than the fix itself. In particular, throws were connected irrespective of
-  the type of the exception
- whether the try was surrounded by a catch

This in turn caused weird CFGs with dangling and impossible to understand nodes:(

This diff reverts this change for now.

Instead, the fix should probably be done in the frontend where we have more information about try/catch blocks.

Reviewed By: da319

Differential Revision: D25997475

fbshipit-source-id: bbeabfbef
master
Ezgi Çiçek 4 years ago committed by Facebook GitHub Bot
parent 1bce54aaf3
commit 840a10afaa

@ -450,49 +450,12 @@ module NoReturn = struct
false )
let has_throw_call node =
Procdesc.Node.get_instrs node
|> Instrs.exists ~f:(fun (instr : Sil.instr) ->
match instr with
| Call (_, Const (Cfun proc_name), _, _, _) ->
String.equal
(Procname.get_method BuiltinDecl.objc_cpp_throw)
(Procname.get_method proc_name)
| _ ->
false )
let get_all_reachable_catch_nodes start_node =
let rec worklist ~todo ~visited result =
if Procdesc.NodeSet.is_empty todo then result
else
let el = Procdesc.NodeSet.choose todo in
let todo = Procdesc.NodeSet.remove el todo in
if Procdesc.NodeSet.mem el visited then worklist ~todo ~visited result
else
let succs = Procdesc.Node.get_succs el |> Procdesc.NodeSet.of_list in
let visited = Procdesc.NodeSet.add el visited in
worklist
~todo:(Procdesc.NodeSet.union succs todo)
~visited
(Procdesc.Node.get_exn el @ result)
in
worklist ~todo:(Procdesc.NodeSet.singleton start_node) ~visited:Procdesc.NodeSet.empty []
let process tenv proc_desc =
let exit_node = Procdesc.get_exit_node proc_desc in
Procdesc.iter_nodes
(fun node ->
if has_noreturn_call tenv node then
Procdesc.set_succs node ~normal:(Some [exit_node]) ~exn:None
else if has_throw_call node then
let catch_nodes = get_all_reachable_catch_nodes node in
let catch_or_exit_nodes =
if List.is_empty catch_nodes then (* throw with no catch *)
[exit_node] else catch_nodes
in
Procdesc.set_succs node ~normal:(Some catch_or_exit_nodes) ~exn:None )
Procdesc.set_succs node ~normal:(Some [exit_node]) ~exn:None )
proc_desc
end

@ -441,8 +441,21 @@ class Exceptions {
return 0;
}
// the transition to the catch block is set at pre-analysis
int read_in_catch_tricky_ok(bool b1, bool b2) {
int FN_throw_unrelated_catch_bad(int x) {
int i = 5;
throw std::invalid_argument("Positive argument :(");
// the rest is unreachable
try {
throw(0);
} catch (...) {
return i;
}
}
// currently, the only transition to the catch block is at the end of the try
// block. See T28898377
int FP_read_in_catch_tricky_ok(bool b1, bool b2) {
int i = 1;
try {
if (b1) {

@ -1,3 +1,4 @@
codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::Exceptions::FP_read_in_catch_tricky_ok, 1, DEAD_STORE, no_bucket, ERROR, [Write of unused value]
codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::Exceptions::dead_in_catch_bad, 4, DEAD_STORE, no_bucket, ERROR, [Write of unused value]
codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::Exceptions::unreachable_catch_bad, 1, DEAD_STORE, no_bucket, ERROR, [Write of unused value]
codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::FP_assign_array_tricky2_ok, 3, DEAD_STORE, no_bucket, ERROR, [Write of unused value]

Loading…
Cancel
Save