-
Notifications
You must be signed in to change notification settings - Fork 273
Fix lazy loading of enum static initializers #2754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
public enum Color { | ||
RED, | ||
GREEN, | ||
BLUE | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
public class MyClass { | ||
|
||
private Color c; | ||
|
||
public int myMethod() { | ||
if (c == null) | ||
return 10; | ||
return 20; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
public class NondetEnumArgField { | ||
|
||
public static void test(MyClass mc) { | ||
int result = mc.myMethod(); | ||
assert result == 10; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
NondetEnumArgField.class | ||
--function NondetEnumArgField.test --cp `../../../../scripts/format_classpath.sh . ../../../src/java_bytecode/library/core-models.jar` | ||
^VERIFICATION FAILED$ | ||
-- | ||
-- | ||
The test checks that even when none of the enum constants are referenced in user | ||
code, a nondet enum (in this case, a field of an argument) is still correctly | ||
loaded and initialized. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -382,16 +382,6 @@ void ci_lazy_methodst::initialize_instantiated_classes( | |
if(param.type().id()==ID_pointer) | ||
{ | ||
const pointer_typet &original_pointer = to_pointer_type(param.type()); | ||
const auto &original_type = ns.follow(original_pointer.subtype()); | ||
// Special case for enums. We may want to generalise this, see TG-4689 | ||
// and the comment in java_object_factoryt::gen_nondet_pointer_init. | ||
if( | ||
can_cast_type<java_class_typet>(original_type) && | ||
to_java_class_type(original_type).get_base("java::java.lang.Enum")) | ||
{ | ||
add_clinit_call_for_pointer_type( | ||
original_pointer, ns.get_symbol_table(), needed_lazy_methods); | ||
} | ||
needed_lazy_methods.add_all_needed_classes(original_pointer); | ||
} | ||
} | ||
|
@@ -407,28 +397,35 @@ void ci_lazy_methodst::initialize_instantiated_classes( | |
// As in class_loader, ensure these classes stay available | ||
for(const auto &id : extra_instantiated_classes) | ||
needed_lazy_methods.add_needed_class("java::" + id2string(id)); | ||
|
||
// Special case for enums. We may want to generalise this, see TG-4689 | ||
// and the comment in java_object_factoryt::gen_nondet_pointer_init. | ||
for(const auto &found_class : needed_lazy_methods.get_instantiated_classes()) | ||
{ | ||
const auto &class_type = to_java_class_type(ns.lookup(found_class).type); | ||
if(class_type.get_base("java::java.lang.Enum")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can test on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Huh, I've been doing this all over the place and no one told me about |
||
add_clinit_call(found_class, ns.get_symbol_table(), needed_lazy_methods); | ||
} | ||
} | ||
|
||
/// Helper function for `initialize_instantiated_classes`. | ||
/// For a given pointer_typet that is being noted as needed in | ||
/// `needed_lazy_methods`, notes that its static initializer is also needed. | ||
/// This applies the same logic to the class of `pointer_type` that | ||
/// For a given class id that is being noted as needed in `needed_lazy_methods`, | ||
/// notes that its static initializer is also needed. | ||
/// This applies the same logic to the given class that | ||
/// `java_bytecode_convert_methodt::get_clinit_call` applies e.g. to classes | ||
/// whose constructor we call in a method body. This duplication is unavoidable | ||
/// due to the fact that ci_lazy_methods essentially has to go through the same | ||
/// logic as __CPROVER_start in its initial setup. | ||
/// \param pointer_type: The given pointer_typet | ||
/// \param class_id: The given class id | ||
/// \param symbol_table: Used to look up occurrences of static initializers | ||
/// \param [out] needed_lazy_methods: Gets notified of any static initializers | ||
/// that need to be loaded | ||
void ci_lazy_methodst::add_clinit_call_for_pointer_type( | ||
const pointer_typet &pointer_type, | ||
void ci_lazy_methodst::add_clinit_call( | ||
const irep_idt &class_id, | ||
const symbol_tablet &symbol_table, | ||
ci_lazy_methods_neededt &needed_lazy_methods) | ||
{ | ||
const irep_idt &pointer_id = | ||
to_symbol_type(pointer_type.subtype()).get_identifier(); | ||
const irep_idt &clinit_wrapper = clinit_wrapper_name(pointer_id); | ||
const irep_idt &clinit_wrapper = clinit_wrapper_name(class_id); | ||
if(symbol_table.symbols.count(clinit_wrapper)) | ||
needed_lazy_methods.add_needed_method(clinit_wrapper); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why can't the field be
null
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can, I just wanted it to assert that it can in fact come up with a non-null enum.