|
|
|
|
/* @generated */
|
|
|
|
|
digraph cfg {
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_1" [label="1: Start FN_deref_null_after_catch_bad\nFormals: i:int*\nLocals: 0$?%__sil_tmp__temp_construct_n$6:std::runtime_error 0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_1" -> "FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_7" ;
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_2" [label="2: Exit FN_deref_null_after_catch_bad \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_3" [label="3: Return Stmt \n n$0=*&i:int* [line 52, column 11]\n n$1=*n$0:int [line 52, column 10]\n *&return:int=n$1 [line 52, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_3" -> "FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_2" ;
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_4" [label="4: Destruction(temporaries cleanup) \n _=*&0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const [line 48, column 37]\n n$4=_fun_std::runtime_error::~runtime_error(&0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const *) injected virtual [line 48, column 37]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_4" -> "FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_3" ;
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_4" -> "FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_8" [color="red" ];
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_5" [label="5: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const ); [line 48, column 11]\n n$7=_fun_std::runtime_error::runtime_error(&0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const *,\"error\":char const *) [line 48, column 11]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_5" -> "FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_6" ;
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_6" [label="6: ObjCCPPThrow \n n$8=_fun_std::runtime_error::runtime_error(&0$?%__sil_tmp__temp_construct_n$6:std::runtime_error*,&0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const &) [line 48, column 11]\n n$9=_fun___infer_objc_cpp_throw(&0$?%__sil_tmp__temp_construct_n$6:std::runtime_error) [line 48, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_6" -> "FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_4" ;
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_7" [label="7: BinaryOperatorStmt: Assign \n n$10=*&i:int* [line 47, column 6]\n *n$10:int=2 [line 47, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_7" -> "FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_5" ;
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_8" [label="8: BinaryOperatorStmt: Assign \n *&i:int*=null [line 50, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_8" -> "FN_deref_null_after_catch_bad#4627123003703707696.43441e3badf1bb571cbe770f9d51a51c_3" ;
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_1" [label="1: Start FN_deref_null_in_catch_bad\nFormals: \nLocals: 0$?%__sil_tmp__temp_construct_n$4:std::runtime_error 0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const i:int* \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_1" -> "FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_8" ;
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_2" [label="2: Exit FN_deref_null_in_catch_bad \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_3" [label="3: Return Stmt \n *&return:int=0 [line 42, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_3" -> "FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_2" ;
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_4" [label="4: Destruction(temporaries cleanup) \n _=*&0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const [line 38, column 37]\n n$2=_fun_std::runtime_error::~runtime_error(&0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const *) injected virtual [line 38, column 37]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_4" -> "FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_3" ;
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_4" -> "FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_7" [color="red" ];
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_5" [label="5: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const ); [line 38, column 11]\n n$5=_fun_std::runtime_error::runtime_error(&0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const *,\"error\":char const *) [line 38, column 11]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_5" -> "FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_6" ;
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_6" [label="6: ObjCCPPThrow \n n$6=_fun_std::runtime_error::runtime_error(&0$?%__sil_tmp__temp_construct_n$4:std::runtime_error*,&0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const &) [line 38, column 11]\n n$7=_fun___infer_objc_cpp_throw(&0$?%__sil_tmp__temp_construct_n$4:std::runtime_error) [line 38, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_6" -> "FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_4" ;
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_7" [label="7: Return Stmt \n n$8=*&i:int* [line 40, column 13]\n n$9=*n$8:int [line 40, column 12]\n *&return:int=n$9 [line 40, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_7" -> "FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_2" ;
|
|
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_8" [label="8: DeclStmt \n VARIABLE_DECLARED(i:int*); [line 36, column 3]\n *&i:int*=null [line 36, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_8" -> "FN_deref_null_in_catch_bad#9297890526029657977.c83eec7c9ab8ce2e38ddbc08f8c3dfeb_5" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_1" [label="1: Start FN_multiple_catches_bad\nFormals: b:_Bool\nLocals: 0$?%__sil_tmp__temp_construct_n$5:std::length_error 0$?%__sil_tmpSIL_materialize_temp__n$1:std::length_error const 0$?%__sil_tmp__temp_construct_n$13:std::range_error 0$?%__sil_tmpSIL_materialize_temp__n$9:std::range_error const j:int* i:int* \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_1" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_16" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_2" [label="2: Exit FN_multiple_catches_bad \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_3" [label="3: Return Stmt \n *&return:int=0 [line 69, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_3" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_2" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_4" [label="4: + \n " ]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_4" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_3" ;
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_4" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_14" [color="red" ];
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_4" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_13" [color="red" ];
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_5" [label="5: Prune (true branch, if) \n n$0=*&b:_Bool [line 59, column 9]\n PRUNE(n$0, true); [line 59, column 9]\n " shape="invhouse"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_5" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_8" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_6" [label="6: Prune (false branch, if) \n n$0=*&b:_Bool [line 59, column 9]\n PRUNE(!n$0, false); [line 59, column 9]\n " shape="invhouse"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_6" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_11" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_7" [label="7: Destruction(temporaries cleanup) \n _=*&0$?%__sil_tmpSIL_materialize_temp__n$1:std::length_error const [line 60, column 38]\n n$3=_fun_std::length_error::~length_error(&0$?%__sil_tmpSIL_materialize_temp__n$1:std::length_error const *) injected virtual [line 60, column 38]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_7" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_4" ;
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_8" [label="8: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$1:std::length_error const ); [line 60, column 13]\n n$6=_fun_std::length_error::length_error(&0$?%__sil_tmpSIL_materialize_temp__n$1:std::length_error const *,\"error\":char const *) [line 60, column 13]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_8" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_9" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_9" [label="9: ObjCCPPThrow \n n$7=_fun_std::length_error::length_error(&0$?%__sil_tmp__temp_construct_n$5:std::length_error*,&0$?%__sil_tmpSIL_materialize_temp__n$1:std::length_error const &) [line 60, column 13]\n n$8=_fun___infer_objc_cpp_throw(&0$?%__sil_tmp__temp_construct_n$5:std::length_error) [line 60, column 7]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_9" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_7" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_10" [label="10: Destruction(temporaries cleanup) \n _=*&0$?%__sil_tmpSIL_materialize_temp__n$9:std::range_error const [line 62, column 37]\n n$11=_fun_std::range_error::~range_error(&0$?%__sil_tmpSIL_materialize_temp__n$9:std::range_error const *) injected virtual [line 62, column 37]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_10" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_4" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_11" [label="11: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$9:std::range_error const ); [line 62, column 13]\n n$14=_fun_std::range_error::range_error(&0$?%__sil_tmpSIL_materialize_temp__n$9:std::range_error const *,\"error\":char const *) [line 62, column 13]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_11" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_12" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_12" [label="12: ObjCCPPThrow \n n$15=_fun_std::range_error::range_error(&0$?%__sil_tmp__temp_construct_n$13:std::range_error*,&0$?%__sil_tmpSIL_materialize_temp__n$9:std::range_error const &) [line 62, column 13]\n n$16=_fun___infer_objc_cpp_throw(&0$?%__sil_tmp__temp_construct_n$13:std::range_error) [line 62, column 7]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_12" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_10" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_13" [label="13: Return Stmt \n n$18=*&i:int* [line 65, column 13]\n n$19=*n$18:int [line 65, column 12]\n *&return:int=n$19 [line 65, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_13" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_2" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_14" [label="14: Return Stmt \n n$20=*&j:int* [line 67, column 13]\n n$21=*n$20:int [line 67, column 12]\n *&return:int=n$21 [line 67, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_14" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_2" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_15" [label="15: DeclStmt \n VARIABLE_DECLARED(j:int*); [line 57, column 3]\n *&j:int*=null [line 57, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_15" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_5" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_15" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_6" ;
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_16" [label="16: DeclStmt \n VARIABLE_DECLARED(i:int*); [line 56, column 3]\n *&i:int*=null [line 56, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_16" -> "FN_multiple_catches_bad#4595182522053295670.680a793e449c2d7439ff6441ca69fa98_15" ;
|
|
|
|
|
"basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_1" [label="1: Start basic_throw_ok\nFormals: \nLocals: 0$?%__sil_tmp__temp_construct_n$4:std::runtime_error 0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_1" -> "basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_4" ;
|
|
|
|
|
"basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_2" [label="2: Exit basic_throw_ok \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_3" [label="3: Destruction(temporaries cleanup) \n _=*&0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const [line 27, column 61]\n n$2=_fun_std::runtime_error::~runtime_error(&0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const *) injected virtual [line 27, column 61]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_3" -> "basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_2" ;
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_4" [label="4: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const ); [line 27, column 31]\n n$5=_fun_std::runtime_error::runtime_error(&0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const *,\"throwing!\":char const *) [line 27, column 31]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_4" -> "basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_5" ;
|
|
|
|
|
"basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_5" [label="5: ObjCCPPThrow \n n$6=_fun_std::runtime_error::runtime_error(&0$?%__sil_tmp__temp_construct_n$4:std::runtime_error*,&0$?%__sil_tmpSIL_materialize_temp__n$0:std::runtime_error const &) [line 27, column 31]\n n$7=_fun___infer_objc_cpp_throw(&0$?%__sil_tmp__temp_construct_n$4:std::runtime_error) [line 27, column 25]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_5" -> "basic_throw_ok#10529188890980782893.c9e1b8dd080b2621cfca65612331859d_3" ;
|
|
|
|
|
"call_deref_with_null#4611966425999531792.6346543307e9a799421a89e451b917c2_1" [label="1: Start call_deref_with_null\nFormals: \nLocals: \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"call_deref_with_null#4611966425999531792.6346543307e9a799421a89e451b917c2_1" -> "call_deref_with_null#4611966425999531792.6346543307e9a799421a89e451b917c2_3" ;
|
|
|
|
|
"call_deref_with_null#4611966425999531792.6346543307e9a799421a89e451b917c2_2" [label="2: Exit call_deref_with_null \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"call_deref_with_null#4611966425999531792.6346543307e9a799421a89e451b917c2_3" [label="3: Call _fun_deref_null \n n$0=_fun_deref_null(null:int*) [line 25, column 30]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"call_deref_with_null#4611966425999531792.6346543307e9a799421a89e451b917c2_3" -> "call_deref_with_null#4611966425999531792.6346543307e9a799421a89e451b917c2_2" ;
|
|
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_1" [label="1: Start dead_deref_null_after_throw_ok\nFormals: \nLocals: 0$?%__sil_tmp__temp_construct_n$6:std::runtime_error 0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const i:int* \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_1" -> "dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_7" ;
|
|
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_2" [label="2: Exit dead_deref_null_after_throw_ok \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_3" [label="3: Return Stmt \n n$0=*&i:int* [line 32, column 11]\n n$1=*n$0:int [line 32, column 10]\n *&return:int=n$1 [line 32, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_3" -> "dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_2" ;
|
|
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_4" [label="4: Destruction(temporaries cleanup) \n _=*&0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const [line 31, column 39]\n n$4=_fun_std::runtime_error::~runtime_error(&0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const *) injected virtual [line 31, column 39]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_4" -> "dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_3" ;
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_5" [label="5: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const ); [line 31, column 9]\n n$7=_fun_std::runtime_error::runtime_error(&0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const *,\"throwing!\":char const *) [line 31, column 9]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_5" -> "dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_6" ;
|
|
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_6" [label="6: ObjCCPPThrow \n n$8=_fun_std::runtime_error::runtime_error(&0$?%__sil_tmp__temp_construct_n$6:std::runtime_error*,&0$?%__sil_tmpSIL_materialize_temp__n$2:std::runtime_error const &) [line 31, column 9]\n n$9=_fun___infer_objc_cpp_throw(&0$?%__sil_tmp__temp_construct_n$6:std::runtime_error) [line 31, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_6" -> "dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_4" ;
|
|
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_7" [label="7: DeclStmt \n VARIABLE_DECLARED(i:int*); [line 30, column 3]\n *&i:int*=null [line 30, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_7" -> "dead_deref_null_after_throw_ok#12025371096822526715.42d41c040f3a321bb94f60bf7b55d001_5" ;
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_1" [label="1: Start deref\nFormals: p:int*\nLocals: \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_1" -> "deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_5" ;
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_2" [label="2: Exit deref \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_3" [label="3: Return Stmt \n n$0=*&p:int* [line 15, column 11]\n n$1=*n$0:int [line 15, column 10]\n *&return:int=n$1 [line 15, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_3" -> "deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_2" ;
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_4" [label="4: + \n " ]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_4" -> "deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_3" ;
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_5" [label="5: BinaryOperatorStmt: EQ \n n$2=*&p:int* [line 12, column 7]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_5" -> "deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_6" ;
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_5" -> "deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_7" ;
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_6" [label="6: Prune (true branch, if) \n PRUNE((n$2 == null), true); [line 12, column 7]\n " shape="invhouse"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_6" -> "deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_8" ;
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_7" [label="7: Prune (false branch, if) \n PRUNE(!(n$2 == null), false); [line 12, column 7]\n " shape="invhouse"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_7" -> "deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_4" ;
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_8" [label="8: ObjCCPPThrow \n n$3=_fun___infer_objc_cpp_throw(\"Null pointer!\":char const *) [line 13, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_8" -> "deref#13506892413034678690.824465c4193ad2288eb512b1083edab3_4" ;
|
|
|
|
|
"deref_null#11536394632240553702.ea4eed042da22ab7ceb619ec1b7f73bb_1" [label="1: Start deref_null\nFormals: p:int*\nLocals: \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref_null#11536394632240553702.ea4eed042da22ab7ceb619ec1b7f73bb_1" -> "deref_null#11536394632240553702.ea4eed042da22ab7ceb619ec1b7f73bb_3" ;
|
|
|
|
|
"deref_null#11536394632240553702.ea4eed042da22ab7ceb619ec1b7f73bb_2" [label="2: Exit deref_null \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref_null#11536394632240553702.ea4eed042da22ab7ceb619ec1b7f73bb_3" [label="3: Return Stmt \n n$0=*&p:int* [line 20, column 13]\n n$1=*n$0:int [line 20, column 12]\n *&return:int=n$1 [line 20, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"deref_null#11536394632240553702.ea4eed042da22ab7ceb619ec1b7f73bb_3" -> "deref_null#11536394632240553702.ea4eed042da22ab7ceb619ec1b7f73bb_2" ;
|
|
|
|
|
"main.fad58de7366495db4650cfefac2fcd61_1" [label="1: Start main\nFormals: \nLocals: \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"main.fad58de7366495db4650cfefac2fcd61_1" -> "main.fad58de7366495db4650cfefac2fcd61_3" ;
|
|
|
|
|
"main.fad58de7366495db4650cfefac2fcd61_2" [label="2: Exit main \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"main.fad58de7366495db4650cfefac2fcd61_3" [label="3: Return Stmt \n n$0=_fun_deref(null:int*) [line 74, column 12]\n *&return:int=n$0 [line 74, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"main.fad58de7366495db4650cfefac2fcd61_3" -> "main.fad58de7366495db4650cfefac2fcd61_2" ;
|
|
|
|
|
"main.fad58de7366495db4650cfefac2fcd61_4" [label="4: Return Stmt \n *&return:int=-1 [line 76, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"main.fad58de7366495db4650cfefac2fcd61_4" -> "main.fad58de7366495db4650cfefac2fcd61_2" ;
|
|
|
|
|
}
|
