From 4e64657d5037d067589392a100f4f5f09aceb3ab Mon Sep 17 00:00:00 2001 From: Sungkeun Cho Date: Thu, 14 May 2020 00:37:02 -0700 Subject: [PATCH] [frontend][clang][RFC] fix translation of enum constant Summary: Previous translation of enum constants were wrong since they assumed that the enum constant didn't include any global variable (hence they just looked up the enum exp from the map, forgetting to tie the respective instructions into the cfg). ``` const int gvar = 0; enum { evar = gvar, }; int dangling() { return evar; } ``` as a result, the CFG was missing the instruction for the load of the `gvar`. {F237004587} This diff fixes this issue by hooking up the instructions that load the enum constant in to the CFG. Note that in this example, it is only a load instruction but there could be more instructions (e.g. if we had `gvar > 1`, we would have prune +join). {F237004493} Reviewed By: ezgicicek Differential Revision: D21549781 fbshipit-source-id: 525534fb2 --- infer/src/clang/cTrans.ml | 86 +++++++++---------- .../codetoanalyze/c/bufferoverrun/global.c | 2 +- .../codetoanalyze/c/bufferoverrun/issues.exp | 3 +- 3 files changed, 43 insertions(+), 48 deletions(-) diff --git a/infer/src/clang/cTrans.ml b/infer/src/clang/cTrans.ml index a66f372c4..e401bb601 100644 --- a/infer/src/clang/cTrans.ml +++ b/infer/src/clang/cTrans.ml @@ -867,47 +867,47 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s decl_ref_trans ~context:DeclRefExpr trans_state stmt_info decl_ref - (** evaluates an enum constant *) - and enum_const_eval context enum_constant_pointer prev_enum_constant_opt zero = - match CAst_utils.get_decl enum_constant_pointer with - | Some (Clang_ast_t.EnumConstantDecl (_, _, _, enum_constant_decl_info)) -> ( - match enum_constant_decl_info.Clang_ast_t.ecdi_init_expr with - | Some stmt -> - expression_trans context stmt - | None -> ( - match prev_enum_constant_opt with - | Some prev_constant_pointer -> - let previous_exp = get_enum_constant_expr context prev_constant_pointer in - CArithmetic_trans.sil_const_plus_one previous_exp - | None -> - zero ) ) - | _ -> - zero - - - (** get the sil value of the enum constant from the map or by evaluating it *) - and get_enum_constant_expr context enum_constant_pointer = - let zero = Exp.Const (Const.Cint IntLit.zero) in - try - let prev_enum_constant_opt, sil_exp_opt = - CAst_utils.get_enum_constant_exp_exn enum_constant_pointer - in - match sil_exp_opt with - | Some exp -> - exp - | None -> - let exp = enum_const_eval context enum_constant_pointer prev_enum_constant_opt zero in - CAst_utils.update_enum_map_exn enum_constant_pointer exp ; - exp - with Not_found_s _ | Caml.Not_found -> zero - - and enum_constant_trans trans_state decl_ref = - let context = trans_state.context in - let _, _, qual_type = CAst_utils.get_info_from_decl_ref decl_ref in - let typ = CType_decl.qual_type_to_sil_type context.CContext.tenv qual_type in - let const_exp = get_enum_constant_expr context decl_ref.Clang_ast_t.dr_decl_pointer in - mk_trans_result (const_exp, typ) empty_control + let typ = + let {CContext.tenv} = trans_state.context in + let _, _, qual_type = CAst_utils.get_info_from_decl_ref decl_ref in + CType_decl.qual_type_to_sil_type tenv qual_type + in + let zero () = mk_trans_result (Exp.Const (Const.Cint IntLit.zero), typ) empty_control in + (* translate enum declaration statements *) + let rec trans_enum_decl ~prev_decl_pointer_opt decl_pointer = + match CAst_utils.get_decl decl_pointer with + | Some (Clang_ast_t.EnumConstantDecl (_, _, _, enum_constant_decl_info)) -> ( + match enum_constant_decl_info.Clang_ast_t.ecdi_init_expr with + | Some stmt -> + instruction trans_state stmt + | None -> ( + match prev_decl_pointer_opt with + | Some prev_decl_pointer -> + let ({return= exp, typ} as trans_result) = trans_with_cache prev_decl_pointer in + {trans_result with return= (CArithmetic_trans.sil_const_plus_one exp, typ)} + | None -> + zero () ) ) + | _ -> + zero () + (* try looking up from cache before calling [trans_enum_decl] *) + and trans_with_cache decl_pointer = + try + let prev_decl_pointer_opt, cached_exp = CAst_utils.get_enum_constant_exp_exn decl_pointer in + match cached_exp with + | Some exp -> + mk_trans_result (exp, typ) empty_control + | None -> + let trans_result = trans_enum_decl ~prev_decl_pointer_opt decl_pointer in + ( match fst trans_result.return with + | Exp.Const _ as exp -> + CAst_utils.update_enum_map_exn decl_pointer exp + | _ -> + () ) ; + trans_result + with Not_found_s _ | Caml.Not_found -> zero () + in + trans_with_cache decl_ref.Clang_ast_t.dr_decl_pointer and arraySubscriptExpr_trans trans_state expr_info stmt_list = @@ -3898,12 +3898,6 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s exec_trans_instrs trans_state stmt_trans_fun - and expression_trans context stmt = - let trans_state = CTrans_utils.default_trans_state context in - let res_trans_stmt = instruction trans_state stmt in - fst res_trans_stmt.return - - let instructions_trans context body extra_instrs exit_node ~is_destructor_wrapper = let default_trans_state = CTrans_utils.default_trans_state context in let trans_state = {default_trans_state with succ_nodes= [exit_node]} in diff --git a/infer/tests/codetoanalyze/c/bufferoverrun/global.c b/infer/tests/codetoanalyze/c/bufferoverrun/global.c index 6468ea088..4cf3dd4d3 100644 --- a/infer/tests/codetoanalyze/c/bufferoverrun/global.c +++ b/infer/tests/codetoanalyze/c/bufferoverrun/global.c @@ -22,7 +22,7 @@ void compare_global_const_enum_Bad() { arr[10] = 1; } -void compare_global_const_enum_Good_FP() { +void compare_global_const_enum_Good() { char arr[10]; if (global_const > 10) arr[10] = 1; diff --git a/infer/tests/codetoanalyze/c/bufferoverrun/issues.exp b/infer/tests/codetoanalyze/c/bufferoverrun/issues.exp index b5df0c09b..41d8a5607 100644 --- a/infer/tests/codetoanalyze/c/bufferoverrun/issues.exp +++ b/infer/tests/codetoanalyze/c/bufferoverrun/issues.exp @@ -127,8 +127,9 @@ codetoanalyze/c/bufferoverrun/get_field.c, call_get_v2_Bad, 8, BUFFER_OVERRUN_L5 codetoanalyze/c/bufferoverrun/get_field.c, call_get_v2_Good_FP, 8, BUFFER_OVERRUN_L5, no_bucket, ERROR, [,Call,Assignment,,Array declaration,Array access: Offset: [-oo, +oo] Size: 10] codetoanalyze/c/bufferoverrun/get_field.c, call_get_v_Bad, 8, BUFFER_OVERRUN_L1, no_bucket, ERROR, [,Assignment,Call,Parameter `l->next->prev->v`,Assignment,,Array declaration,Array access: Offset: 10 Size: 10] codetoanalyze/c/bufferoverrun/get_field.c, make_many_locations, 1, CONDITION_ALWAYS_TRUE, no_bucket, WARNING, [Here] +codetoanalyze/c/bufferoverrun/global.c, compare_global_const_enum_Bad, 2, CONDITION_ALWAYS_TRUE, no_bucket, WARNING, [Here] codetoanalyze/c/bufferoverrun/global.c, compare_global_const_enum_Bad, 3, BUFFER_OVERRUN_L1, no_bucket, ERROR, [,Array declaration,Array access: Offset: 10 Size: 10] -codetoanalyze/c/bufferoverrun/global.c, compare_global_const_enum_Good_FP, 3, BUFFER_OVERRUN_L1, no_bucket, ERROR, [,Array declaration,Array access: Offset: 10 Size: 10] +codetoanalyze/c/bufferoverrun/global.c, compare_global_const_enum_Good, 2, CONDITION_ALWAYS_FALSE, no_bucket, WARNING, [Here] codetoanalyze/c/bufferoverrun/global.c, compare_global_variable_bad, 3, BUFFER_OVERRUN_L1, no_bucket, ERROR, [,Array declaration,Array access: Offset: 10 Size: 10] codetoanalyze/c/bufferoverrun/global.c, copyfilter_Good_FP, 3, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [,Assignment,Binary operation: ([0, +oo] + 1):signed32] codetoanalyze/c/bufferoverrun/global.c, use_global_const_ten_Bad, 2, BUFFER_OVERRUN_L1, no_bucket, ERROR, [,Assignment,,Array declaration,Array access: Offset: 10 Size: 5]