Skip to content

Commit f5f00d1

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 1505c36 commit f5f00d1

File tree

8 files changed

+128
-6
lines changed

8 files changed

+128
-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: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ 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
11+
NullPointerException. The null check is now performed during bytecode
12+
conversion.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
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
12+
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: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2108,15 +2108,61 @@ 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
2116+
// NullPointerException 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+
{std::move(assign_new), code_expressiont{std::move(throw_expr)}}};
2151+
2152+
// Create the monitor function call
21122153
java_method_typet type(
21132154
{java_method_typet::parametert(java_reference_type(java_void_type()))},
21142155
java_void_type());
2115-
code_function_callt call(symbol_exprt(descriptor, type), {op[0]});
2156+
code_function_callt call{symbol_exprt(descriptor, type), {objectref}};
21162157
call.add_source_location() = source_location;
21172158
if(needed_lazy_methods && symbol_table.has_symbol(descriptor))
21182159
needed_lazy_methods->add_needed_method(descriptor);
2119-
return std::move(call);
2160+
2161+
// Return if-then-else: if null, throw exception; else, call monitor function
2162+
code_ifthenelset if_code{is_null, throw_block, call};
2163+
if_code.add_source_location() = source_location;
2164+
2165+
return std::move(if_code);
21202166
}
21212167

21222168
void java_bytecode_convert_methodt::convert_dup2(

0 commit comments

Comments
 (0)