Skip to content

Commit 85b1e4f

Browse files
committed
Invoke static initializers on demand
Previously these were run from CPROVER_init; now they are run on demand, using the same rules as the JVM (in brief, when `new` is called, or a static field accessed, or a static method called). This means that a faulting static initializer no longer blocks all coverage, instead blocking only paths that would necessarily call the problematic initializer, and initializers are called in the correct order, which they sometimes depend upon (e.g. a class initializer assuming an enumeration initializer has run before it uses the `.values()` method) For details, see http://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.4 This commit also amends the covered1 test, whose precise results I think are still correct, but are perturbed by this change.
1 parent 668f30f commit 85b1e4f

File tree

4 files changed

+211
-61
lines changed

4 files changed

+211
-61
lines changed

regression/cbmc-java/covered1/test.desc

+1-2
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,8 @@ covered1.class
1111
.*\"coveredLines\": \"28\",$
1212
.*\"coveredLines\": \"28\",$
1313
.*\"coveredLines\": \"29\",$
14-
.*\"coveredLines\": \"9,18\",$
1514
.*\"coveredLines\": \"18\",$
1615
.*\"coveredLines\": \"18\",$
17-
.*\"coveredLines\": \"18,35\",$
16+
.*\"coveredLines\": \"35\",$
1817
--
1918
^warning: ignoring

src/java_bytecode/java_bytecode_convert_method.cpp

+204-2
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,174 @@ void java_bytecode_convert_methodt::check_static_field_stub(
911911

912912
/*******************************************************************\
913913
914+
Function: java_bytecode_convert_methodt::class_needs_clinit
915+
916+
Inputs: classname: Class name
917+
918+
Outputs: Returns true if the given class or one of its parents
919+
has a static initializer
920+
921+
Purpose: Determine whether a `new` or static access against `classname`
922+
should be prefixed with a static initialization check.
923+
924+
\*******************************************************************/
925+
926+
bool java_bytecode_convert_methodt::class_needs_clinit(
927+
const irep_idt &classname)
928+
{
929+
auto findit_any=any_superclass_has_clinit_method.insert({classname, false});
930+
if(!findit_any.second)
931+
return findit_any.first->second;
932+
933+
auto findit_here=class_has_clinit_method.insert({classname, false});
934+
if(findit_here.second)
935+
{
936+
const irep_idt &clinit_name=id2string(classname)+".<clinit>:()V";
937+
findit_here.first->second=symbol_table.symbols.count(clinit_name);
938+
}
939+
if(findit_here.first->second)
940+
{
941+
findit_any.first->second=true;
942+
return true;
943+
}
944+
auto findit_symbol=symbol_table.symbols.find(classname);
945+
// Stub class?
946+
if(findit_symbol==symbol_table.symbols.end())
947+
{
948+
warning() << "SKIPPED: " << classname << eom;
949+
return false;
950+
}
951+
const symbolt &class_symbol=symbol_table.lookup(classname);
952+
for(const auto &base : to_class_type(class_symbol.type).bases())
953+
{
954+
if(class_needs_clinit(to_symbol_type(base.type()).get_identifier()))
955+
{
956+
findit_any.first->second=true;
957+
return true;
958+
}
959+
}
960+
return false;
961+
}
962+
963+
/*******************************************************************\
964+
965+
Function: java_bytecode_convert_methodt::get_or_create_clinit_wrapper
966+
967+
Inputs: classname: Class name
968+
969+
Outputs: Returns a symbol_exprt pointing to the given class' clinit
970+
wrapper if one is required, or nil otherwise.
971+
972+
Purpose: Create a ::clinit_wrapper the first time a static initializer
973+
might be called. The wrapper method checks whether static init
974+
has already taken place, calls the actual <clinit> method if
975+
not, and initializes super-classes and interfaces.
976+
977+
\*******************************************************************/
978+
979+
exprt java_bytecode_convert_methodt::get_or_create_clinit_wrapper(
980+
const irep_idt &classname)
981+
{
982+
if(!class_needs_clinit(classname))
983+
return static_cast<const exprt &>(get_nil_irep());
984+
985+
const irep_idt &clinit_wrapper_name=
986+
id2string(classname)+"::clinit_wrapper";
987+
auto findit=symbol_table.symbols.find(clinit_wrapper_name);
988+
if(findit!=symbol_table.symbols.end())
989+
return findit->second.symbol_expr();
990+
991+
// Create the wrapper now:
992+
const irep_idt &already_run_name=
993+
id2string(classname)+"::clinit_already_run";
994+
symbolt already_run_symbol;
995+
already_run_symbol.name=already_run_name;
996+
already_run_symbol.pretty_name=already_run_name;
997+
already_run_symbol.base_name="clinit_already_run";
998+
already_run_symbol.type=bool_typet();
999+
already_run_symbol.value=false_exprt();
1000+
already_run_symbol.is_lvalue=true;
1001+
already_run_symbol.is_state_var=true;
1002+
already_run_symbol.is_static_lifetime=true;
1003+
already_run_symbol.mode=ID_java;
1004+
symbol_table.add(already_run_symbol);
1005+
1006+
equal_exprt check_already_run(
1007+
already_run_symbol.symbol_expr(),
1008+
false_exprt());
1009+
1010+
code_ifthenelset wrapper_body;
1011+
wrapper_body.cond()=check_already_run;
1012+
code_blockt init_body;
1013+
// Note already-run is set *before* calling clinit, in order to prevent
1014+
// recursion in clinit methods.
1015+
code_assignt set_already_run(already_run_symbol.symbol_expr(), true_exprt());
1016+
init_body.move_to_operands(set_already_run);
1017+
const irep_idt &real_clinit_name=id2string(classname)+".<clinit>:()V";
1018+
const symbolt &class_symbol=symbol_table.lookup(classname);
1019+
1020+
auto findsymit=symbol_table.symbols.find(real_clinit_name);
1021+
if(findsymit!=symbol_table.symbols.end())
1022+
{
1023+
code_function_callt call_real_init;
1024+
call_real_init.function()=findsymit->second.symbol_expr();
1025+
init_body.move_to_operands(call_real_init);
1026+
}
1027+
1028+
for(const auto &base : to_class_type(class_symbol.type).bases())
1029+
{
1030+
const auto base_name=to_symbol_type(base.type()).get_identifier();
1031+
exprt base_init_routine=get_or_create_clinit_wrapper(base_name);
1032+
if(base_init_routine.is_nil())
1033+
continue;
1034+
code_function_callt call_base;
1035+
call_base.function()=base_init_routine;
1036+
init_body.move_to_operands(call_base);
1037+
}
1038+
1039+
wrapper_body.then_case()=init_body;
1040+
1041+
symbolt wrapper_method_symbol;
1042+
code_typet wrapper_method_type;
1043+
wrapper_method_type.return_type()=void_typet();
1044+
wrapper_method_symbol.name=clinit_wrapper_name;
1045+
wrapper_method_symbol.pretty_name=clinit_wrapper_name;
1046+
wrapper_method_symbol.base_name="clinit_wrapper";
1047+
wrapper_method_symbol.type=wrapper_method_type;
1048+
wrapper_method_symbol.value=wrapper_body;
1049+
wrapper_method_symbol.mode=ID_java;
1050+
symbol_table.add(wrapper_method_symbol);
1051+
return wrapper_method_symbol.symbol_expr();
1052+
}
1053+
1054+
/*******************************************************************\
1055+
1056+
Function: java_bytecode_convert_methodt::get_clinit_call
1057+
1058+
Inputs: classname: Class name
1059+
1060+
Outputs: Returns a function call to the given class' static initializer
1061+
wrapper if one is needed, or a skip instruction otherwise.
1062+
1063+
Purpose: Each static access to classname should be prefixed with a check
1064+
for necessary static init; this returns a call implementing
1065+
that check.
1066+
1067+
\*******************************************************************/
1068+
1069+
codet java_bytecode_convert_methodt::get_clinit_call(
1070+
const irep_idt &classname)
1071+
{
1072+
exprt callee=get_or_create_clinit_wrapper(classname);
1073+
if(callee.is_nil())
1074+
return code_skipt();
1075+
code_function_callt ret;
1076+
ret.function()=callee;
1077+
return ret;
1078+
}
1079+
1080+
/*******************************************************************\
1081+
9141082
Function: java_bytecode_convert_methodt::convert_instructions
9151083
9161084
Inputs:
@@ -1378,6 +1546,18 @@ codet java_bytecode_convert_methodt::convert_instructions(
13781546

13791547
call.function().add_source_location()=loc;
13801548
c=call;
1549+
1550+
if(!use_this)
1551+
{
1552+
codet clinit_call=get_clinit_call(arg0.get(ID_C_class));
1553+
if(clinit_call.get_statement()!=ID_skip)
1554+
{
1555+
code_blockt ret_block;
1556+
ret_block.move_to_operands(clinit_call);
1557+
ret_block.move_to_operands(c);
1558+
c=std::move(ret_block);
1559+
}
1560+
}
13811561
}
13821562
else if(statement=="return")
13831563
{
@@ -1916,9 +2096,14 @@ codet java_bytecode_convert_methodt::convert_instructions(
19162096
}
19172097
results[0]=java_bytecode_promotion(symbol_expr);
19182098

1919-
// set $assertionDisabled to false
1920-
if(field_name.find("$assertionsDisabled")!=std::string::npos)
2099+
codet clinit_call=get_clinit_call(arg0.get_string(ID_class));
2100+
if(clinit_call.get_statement()!=ID_skip)
2101+
c=clinit_call;
2102+
else if(field_name.find("$assertionsDisabled")!=std::string::npos)
2103+
{
2104+
// set $assertionDisabled to false
19212105
c=code_assignt(symbol_expr, false_exprt());
2106+
}
19222107
}
19232108
else if(statement=="putfield")
19242109
{
@@ -1937,6 +2122,14 @@ codet java_bytecode_convert_methodt::convert_instructions(
19372122
to_symbol_type(arg0.type()).get_identifier());
19382123
}
19392124
c=code_assignt(symbol_expr, op[0]);
2125+
codet clinit_call=get_clinit_call(arg0.get_string(ID_class));
2126+
if(clinit_call.get_statement()!=ID_skip)
2127+
{
2128+
code_blockt ret_block;
2129+
ret_block.move_to_operands(clinit_call);
2130+
ret_block.move_to_operands(c);
2131+
c=std::move(ret_block);
2132+
}
19402133
}
19412134
else if(statement==patternt("?2?")) // i2c etc.
19422135
{
@@ -1955,6 +2148,15 @@ codet java_bytecode_convert_methodt::convert_instructions(
19552148

19562149
const exprt tmp=tmp_variable("new", ref_type);
19572150
c=code_assignt(tmp, java_new_expr);
2151+
codet clinit_call=
2152+
get_clinit_call(to_symbol_type(arg0.type()).get_identifier());
2153+
if(clinit_call.get_statement()!=ID_skip)
2154+
{
2155+
code_blockt ret_block;
2156+
ret_block.move_to_operands(clinit_call);
2157+
ret_block.move_to_operands(c);
2158+
c=std::move(ret_block);
2159+
}
19582160
results[0]=tmp;
19592161
}
19602162
else if(statement=="newarray" ||

src/java_bytecode/java_bytecode_convert_method_class.h

+6
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ class java_bytecode_convert_methodt:public messaget
9494
expanding_vectort<variablest> variables;
9595
std::set<symbol_exprt> used_local_names;
9696
bool method_has_this;
97+
std::map<irep_idt, bool> class_has_clinit_method;
98+
std::map<irep_idt, bool> any_superclass_has_clinit_method;
9799

98100
typedef enum instruction_sizet
99101
{
@@ -221,6 +223,10 @@ class java_bytecode_convert_methodt:public messaget
221223
void check_static_field_stub(
222224
const symbol_exprt &se,
223225
const irep_idt &basename);
226+
227+
bool class_needs_clinit(const irep_idt &classname);
228+
exprt get_or_create_clinit_wrapper(const irep_idt &classname);
229+
codet get_clinit_call(const irep_idt &classname);
224230
};
225231

226232
#endif

src/java_bytecode/java_entry_point.cpp

-57
Original file line numberDiff line numberDiff line change
@@ -98,19 +98,6 @@ Function: java_static_lifetime_init
9898
9999
\*******************************************************************/
100100

101-
struct static_initializer_callt
102-
{
103-
symbol_exprt initializer_symbol;
104-
bool is_enum;
105-
};
106-
107-
static bool static_initializer_call_enum_lt(
108-
const static_initializer_callt &a,
109-
const static_initializer_callt &b)
110-
{
111-
return a.is_enum && !b.is_enum;
112-
}
113-
114101
bool java_static_lifetime_init(
115102
symbol_tablet &symbol_table,
116103
const source_locationt &source_location,
@@ -169,50 +156,6 @@ bool java_static_lifetime_init(
169156
}
170157
}
171158

172-
// we now need to run all the <clinit> methods
173-
174-
std::vector<static_initializer_callt> clinits;
175-
176-
for(symbol_tablet::symbolst::const_iterator
177-
it=symbol_table.symbols.begin();
178-
it!=symbol_table.symbols.end();
179-
it++)
180-
{
181-
if(it->second.base_name=="<clinit>" &&
182-
it->second.type.id()==ID_code &&
183-
it->second.mode==ID_java)
184-
{
185-
const irep_idt symbol_name=
186-
it->second.symbol_expr().get_identifier();
187-
const std::string &symbol_str=id2string(symbol_name);
188-
const std::string suffix(".<clinit>:()V");
189-
assert(has_suffix(symbol_str, suffix));
190-
const std::string class_symbol_name=
191-
symbol_str.substr(0, symbol_str.size()-suffix.size());
192-
const symbolt &class_symbol=symbol_table.lookup(class_symbol_name);
193-
clinits.push_back(
194-
{
195-
it->second.symbol_expr(),
196-
class_symbol.type.get_bool(ID_enumeration)
197-
});
198-
}
199-
}
200-
201-
// Call enumeration initialisers before anything else.
202-
// Really we should be calling them the first time they're referenced,
203-
// as acccording to the JVM spec, but this ought to do the trick for
204-
// most cases with much less effort.
205-
std::sort(clinits.begin(), clinits.end(), static_initializer_call_enum_lt);
206-
207-
for(const auto &clinit : clinits)
208-
{
209-
code_function_callt function_call;
210-
function_call.lhs()=nil_exprt();
211-
function_call.function()=clinit.initializer_symbol;
212-
function_call.add_source_location()=source_location;
213-
code_block.add(function_call);
214-
}
215-
216159
return false;
217160
}
218161

0 commit comments

Comments
 (0)