Skip to content

Commit 060ff98

Browse files
committed
Add NullPointerException for Java synchronized blocks with null monitor
Implement null checking for `monitorenter`/`monitorexit` bytecode instructions to throw `NullPointerException` when the monitor object is null, as per Java bytecode specification. Fixes: #1236
1 parent 4fe3ade commit 060ff98

File tree

8 files changed

+127
-6
lines changed

8 files changed

+127
-6
lines changed
Binary file not shown.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
public class MonitorExitTest {
2+
// This test verifies monitorexit behavior with null
3+
// Note: In normal Java code, you can't call monitorexit directly,
4+
// but the synchronized block generates both monitorenter and monitorexit
5+
public static void main(String[] args) {
6+
final Object o = null;
7+
boolean exceptionCaught = false;
8+
9+
try {
10+
// This will generate both monitorenter and monitorexit bytecodes
11+
synchronized (o) {
12+
// Should not reach here
13+
assert false;
14+
}
15+
// Should not reach here either
16+
assert false;
17+
}
18+
catch (NullPointerException e) {
19+
exceptionCaught = true;
20+
}
21+
22+
// Verify that the exception was caught
23+
assert exceptionCaught : "NullPointerException should have been thrown";
24+
}
25+
}
Binary file not shown.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
public class SyncTest {
2+
// Test that synchronized with null throws NullPointerException
3+
public static void testNull() {
4+
final Object o = null;
5+
try {
6+
synchronized (o) {
7+
assert false; // Should not reach here
8+
}
9+
assert false; // Should not reach here either
10+
}
11+
catch (NullPointerException e) {
12+
// Expected - this is the correct behavior
13+
}
14+
}
15+
16+
// Test that synchronized with non-null object works
17+
public static void testNonNull() {
18+
final Object o = new Object();
19+
try {
20+
synchronized (o) {
21+
// Should execute normally
22+
}
23+
}
24+
catch (NullPointerException e) {
25+
assert false; // Should not throw for non-null object
26+
}
27+
}
28+
29+
public static void main(String[] args) {
30+
testNull();
31+
testNonNull();
32+
}
33+
}

jbmc/regression/jbmc-concurrency/synchronized-blocks-null-throw/test.desc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,5 @@ Sync
77
--
88
^warning: ignoring
99
--
10-
This checks that the synchronized keyword on a null object will always throw.
11-
12-
This is currently not working as explicit throws have been removed from the underlying model due to performance considerations.
10+
This checks that the synchronized keyword on a null object will always throw NullPointerException.
11+
The null check is now performed during bytecode conversion.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
CORE
2+
SyncTest
3+
--cp `../../../../scripts/format_classpath.sh . ../../../lib/java-models-library/target/core-models.jar ../../../lib/java-models-library/target/cprover-api.jar` --java-threading
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
^VERIFICATION SUCCESSFUL$
7+
--
8+
^warning: ignoring
9+
--
10+
Comprehensive test for synchronized blocks with both null and non-null objects.
11+
Verifies that NullPointerException is thrown for null and normal execution for non-null.

jbmc/src/java_bytecode/README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,12 @@ of a synchronized block.
294294
`monitorexit` is converted to a call to
295295
`java::java.lang.Object.monitorexit:(Ljava/lang/Object;)V`.
296296
297+
Before calling these monitor methods, a null check is performed on the object
298+
reference. If the reference is null, a `NullPointerException` is thrown, as
299+
specified by the Java bytecode specification. This ensures that the behavior
300+
matches the JVM specification where both `monitorenter` and `monitorexit`
301+
throw `NullPointerException` when their argument is null.
302+
297303
\subsection converting-synchronized-methods Converting Synchronized Methods
298304
299305
Synchronized methods make it impossible for two invocations of the same method

jbmc/src/java_bytecode/java_bytecode_convert_method.cpp

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2108,15 +2108,62 @@ codet java_bytecode_convert_methodt::convert_monitorenterexit(
21082108
if(!threading_support || !symbol_table.has_symbol(descriptor))
21092109
return code_skipt();
21102110

2111-
// becomes a function call
2111+
PRECONDITION(op.size() == 1);
2112+
const exprt &objectref = op[0];
2113+
2114+
// Create null check for the monitor object
2115+
// According to JVM spec, monitorenter and monitorexit throw NullPointerException
2116+
// if objectref is null
2117+
const equal_exprt is_null(
2118+
objectref, null_pointer_exprt(to_pointer_type(objectref.type())));
2119+
2120+
// Create the NullPointerException throw code
2121+
irep_idt exc_class_name("java::java.lang.NullPointerException");
2122+
2123+
// Ensure the exception class exists
2124+
if(!symbol_table.has_symbol(exc_class_name))
2125+
{
2126+
generate_class_stub(
2127+
"java.lang.NullPointerException",
2128+
symbol_table,
2129+
log.get_message_handler(),
2130+
struct_union_typet::componentst{});
2131+
}
2132+
2133+
pointer_typet exc_ptr_type = pointer_type(struct_tag_typet(exc_class_name));
2134+
2135+
// Allocate and throw an instance of NullPointerException
2136+
symbolt &new_symbol = fresh_java_symbol(
2137+
exc_ptr_type,
2138+
"monitor_null_exception",
2139+
source_location,
2140+
"monitor_null_exception",
2141+
symbol_table);
2142+
2143+
side_effect_exprt new_instance(ID_java_new, exc_ptr_type, source_location);
2144+
code_assignt assign_new(new_symbol.symbol_expr(), new_instance);
2145+
2146+
side_effect_expr_throwt throw_expr(irept(), typet(), source_location);
2147+
throw_expr.copy_to_operands(new_symbol.symbol_expr());
2148+
2149+
code_blockt throw_block;
2150+
throw_block.add(assign_new);
2151+
throw_block.add(code_expressiont(throw_expr));
2152+
2153+
// Create the monitor function call
21122154
java_method_typet type(
21132155
{java_method_typet::parametert(java_reference_type(java_void_type()))},
21142156
java_void_type());
2115-
code_function_callt call(symbol_exprt(descriptor, type), {op[0]});
2157+
code_function_callt call(symbol_exprt(descriptor, type), {objectref});
21162158
call.add_source_location() = source_location;
21172159
if(needed_lazy_methods && symbol_table.has_symbol(descriptor))
21182160
needed_lazy_methods->add_needed_method(descriptor);
2119-
return std::move(call);
2161+
2162+
// Return if-then-else: if null, throw exception; else, call monitor function
2163+
code_ifthenelset if_code(is_null, throw_block, call);
2164+
if_code.add_source_location() = source_location;
2165+
2166+
return std::move(if_code);
21202167
}
21212168

21222169
void java_bytecode_convert_methodt::convert_dup2(

0 commit comments

Comments
 (0)