Read and write constraints for clinit
For clinits executed during pre-compile within a transaction, they are
constrained to only be able to access their own static fields because
the results of iniitialization relying on static fields, either read or
write, of other classes are runtiime determined.
Further discussion can be found at b/62887449.
When SGET and SPUT series instructions are executed, interpreter will
check whether fields are static and does the target match with the
transaction owner, if not, abort transaction and throw an exception.
Test: make test-art-host -j64
Bug: 62887449
Change-Id: I3e655269c8af5f0b3f2c1841fdd2602c827c2757
diff --git a/runtime/common_dex_operations.h b/runtime/common_dex_operations.h
index 528db96..fcc5393 100644
--- a/runtime/common_dex_operations.h
+++ b/runtime/common_dex_operations.h
@@ -200,6 +200,11 @@
break;
}
}
+ if (transaction_active) {
+ if (UNLIKELY(self->IsExceptionPending())) {
+ return false;
+ }
+ }
return true;
}
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index 136d0c6..f8cb243 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -33,6 +33,7 @@
#include "reflection.h"
#include "stack.h"
#include "thread-inl.h"
+#include "transaction.h"
#include "well_known_classes.h"
namespace art {
@@ -42,7 +43,8 @@
ThrowNullPointerExceptionFromDexPC();
}
-template<FindFieldType find_type, Primitive::Type field_type, bool do_access_check>
+template<FindFieldType find_type, Primitive::Type field_type, bool do_access_check,
+ bool transaction_active>
bool DoFieldGet(Thread* self, ShadowFrame& shadow_frame, const Instruction* inst,
uint16_t inst_data) {
const bool is_static = (find_type == StaticObjectRead) || (find_type == StaticPrimitiveRead);
@@ -57,6 +59,13 @@
ObjPtr<mirror::Object> obj;
if (is_static) {
obj = f->GetDeclaringClass();
+ if (transaction_active) {
+ if (Runtime::Current()->GetTransaction()->ReadConstraint(obj.Ptr(), f)) {
+ Runtime::Current()->AbortTransactionAndThrowAbortError(self, "Can't read static fields of "
+ + obj->PrettyTypeOf() + " since it does not belong to clinit's class.");
+ return false;
+ }
+ }
} else {
obj = shadow_frame.GetVRegReference(inst->VRegB_22c(inst_data));
if (UNLIKELY(obj == nullptr)) {
@@ -102,15 +111,17 @@
}
// Explicitly instantiate all DoFieldGet functions.
-#define EXPLICIT_DO_FIELD_GET_TEMPLATE_DECL(_find_type, _field_type, _do_check) \
- template bool DoFieldGet<_find_type, _field_type, _do_check>(Thread* self, \
+#define EXPLICIT_DO_FIELD_GET_TEMPLATE_DECL(_find_type, _field_type, _do_check, _transaction_active) \
+ template bool DoFieldGet<_find_type, _field_type, _do_check, _transaction_active>(Thread* self, \
ShadowFrame& shadow_frame, \
const Instruction* inst, \
uint16_t inst_data)
#define EXPLICIT_DO_FIELD_GET_ALL_TEMPLATE_DECL(_find_type, _field_type) \
- EXPLICIT_DO_FIELD_GET_TEMPLATE_DECL(_find_type, _field_type, false); \
- EXPLICIT_DO_FIELD_GET_TEMPLATE_DECL(_find_type, _field_type, true);
+ EXPLICIT_DO_FIELD_GET_TEMPLATE_DECL(_find_type, _field_type, false, true); \
+ EXPLICIT_DO_FIELD_GET_TEMPLATE_DECL(_find_type, _field_type, false, false); \
+ EXPLICIT_DO_FIELD_GET_TEMPLATE_DECL(_find_type, _field_type, true, true); \
+ EXPLICIT_DO_FIELD_GET_TEMPLATE_DECL(_find_type, _field_type, true, false);
// iget-XXX
EXPLICIT_DO_FIELD_GET_ALL_TEMPLATE_DECL(InstancePrimitiveRead, Primitive::kPrimBoolean)
@@ -261,6 +272,14 @@
ObjPtr<mirror::Object> obj;
if (is_static) {
obj = f->GetDeclaringClass();
+ if (transaction_active) {
+ if (Runtime::Current()->GetTransaction()->WriteConstraint(obj.Ptr(), f)) {
+ Runtime::Current()->AbortTransactionAndThrowAbortError(
+ self, "Can't set fields of " + obj->PrettyTypeOf());
+ return false;
+ }
+ }
+
} else {
obj = shadow_frame.GetVRegReference(inst->VRegB_22c(inst_data));
if (UNLIKELY(obj == nullptr)) {
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index d293aeb..b228e28 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -270,7 +270,8 @@
// Handles iget-XXX and sget-XXX instructions.
// Returns true on success, otherwise throws an exception and returns false.
-template<FindFieldType find_type, Primitive::Type field_type, bool do_access_check>
+template<FindFieldType find_type, Primitive::Type field_type, bool do_access_check,
+ bool transaction_active = false>
bool DoFieldGet(Thread* self, ShadowFrame& shadow_frame, const Instruction* inst,
uint16_t inst_data) REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/interpreter/interpreter_switch_impl.cc b/runtime/interpreter/interpreter_switch_impl.cc
index bdb8332..0c5a45f 100644
--- a/runtime/interpreter/interpreter_switch_impl.cc
+++ b/runtime/interpreter/interpreter_switch_impl.cc
@@ -1313,50 +1313,50 @@
}
case Instruction::SGET_BOOLEAN: {
PREAMBLE();
- bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimBoolean, do_access_check>(
- self, shadow_frame, inst, inst_data);
+ bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimBoolean, do_access_check,
+ transaction_active>(self, shadow_frame, inst, inst_data);
POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_2xx);
break;
}
case Instruction::SGET_BYTE: {
PREAMBLE();
- bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimByte, do_access_check>(
- self, shadow_frame, inst, inst_data);
+ bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimByte, do_access_check,
+ transaction_active>(self, shadow_frame, inst, inst_data);
POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_2xx);
break;
}
case Instruction::SGET_CHAR: {
PREAMBLE();
- bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimChar, do_access_check>(
- self, shadow_frame, inst, inst_data);
+ bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimChar, do_access_check,
+ transaction_active>(self, shadow_frame, inst, inst_data);
POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_2xx);
break;
}
case Instruction::SGET_SHORT: {
PREAMBLE();
- bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimShort, do_access_check>(
- self, shadow_frame, inst, inst_data);
+ bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimShort, do_access_check,
+ transaction_active>(self, shadow_frame, inst, inst_data);
POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_2xx);
break;
}
case Instruction::SGET: {
PREAMBLE();
- bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimInt, do_access_check>(
- self, shadow_frame, inst, inst_data);
+ bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimInt, do_access_check,
+ transaction_active>(self, shadow_frame, inst, inst_data);
POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_2xx);
break;
}
case Instruction::SGET_WIDE: {
PREAMBLE();
- bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimLong, do_access_check>(
- self, shadow_frame, inst, inst_data);
+ bool success = DoFieldGet<StaticPrimitiveRead, Primitive::kPrimLong, do_access_check,
+ transaction_active>(self, shadow_frame, inst, inst_data);
POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_2xx);
break;
}
case Instruction::SGET_OBJECT: {
PREAMBLE();
- bool success = DoFieldGet<StaticObjectRead, Primitive::kPrimNot, do_access_check>(
- self, shadow_frame, inst, inst_data);
+ bool success = DoFieldGet<StaticObjectRead, Primitive::kPrimNot, do_access_check,
+ transaction_active>(self, shadow_frame, inst, inst_data);
POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_2xx);
break;
}
diff --git a/runtime/transaction.cc b/runtime/transaction.cc
index 50deb1f..e923aff 100644
--- a/runtime/transaction.cc
+++ b/runtime/transaction.cc
@@ -119,6 +119,27 @@
return abort_message_;
}
+bool Transaction::WriteConstraint(mirror::Object* obj, ArtField* field) {
+ MutexLock mu(Thread::Current(), log_lock_);
+ if (strict_ // no constraint for boot image
+ && field->IsStatic() // no constraint instance updating
+ && obj != root_) { // modifying other classes' static field, fail
+ return true;
+ }
+ return false;
+}
+
+bool Transaction::ReadConstraint(mirror::Object* obj, ArtField* field) {
+ DCHECK(field->IsStatic());
+ DCHECK(obj->IsClass());
+ MutexLock mu(Thread::Current(), log_lock_);
+ if (!strict_ || // no constraint for boot image
+ obj == root_) { // self-updating, pass
+ return false;
+ }
+ return true;
+}
+
void Transaction::RecordWriteFieldBoolean(mirror::Object* obj,
MemberOffset field_offset,
uint8_t value,
diff --git a/runtime/transaction.h b/runtime/transaction.h
index 64349de..4e9cde5 100644
--- a/runtime/transaction.h
+++ b/runtime/transaction.h
@@ -135,6 +135,14 @@
REQUIRES(!log_lock_)
REQUIRES_SHARED(Locks::mutator_lock_);
+ bool ReadConstraint(mirror::Object* obj, ArtField* field)
+ REQUIRES(!log_lock_)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
+ bool WriteConstraint(mirror::Object* obj, ArtField* field)
+ REQUIRES(!log_lock_)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
private:
class ObjectLog : public ValueObject {
public:
diff --git a/test/660-clinit/expected.txt b/test/660-clinit/expected.txt
index e103a2c..9eb4941 100644
--- a/test/660-clinit/expected.txt
+++ b/test/660-clinit/expected.txt
@@ -1,4 +1,8 @@
JNI_OnLoad called
+A.a: 5
+A.a: 10
+B.b: 10
+C.c: 10
X: 4950
Y: 5730
str: Hello World!
diff --git a/test/660-clinit/src/Main.java b/test/660-clinit/src/Main.java
index f547692..cf2ffe7 100644
--- a/test/660-clinit/src/Main.java
+++ b/test/660-clinit/src/Main.java
@@ -26,7 +26,19 @@
}
expectNotPreInit(Day.class);
- expectNotPreInit(ClInit.class);
+ expectNotPreInit(ClInit.class); // should pass
+ expectNotPreInit(A.class); // should pass
+ expectNotPreInit(B.class); // should fail
+ expectNotPreInit(C.class); // should fail
+
+ A x = new A();
+ System.out.println("A.a: " + A.a);
+
+ B y = new B();
+ C z = new C();
+ System.out.println("A.a: " + A.a);
+ System.out.println("B.b: " + B.b);
+ System.out.println("C.c: " + C.c);
ClInit c = new ClInit();
int aa = c.a;
@@ -113,3 +125,24 @@
}
}
+class A {
+ public static int a = 2;
+ static {
+ a = 5; // self-updating, pass
+ }
+}
+
+class B {
+ public static int b;
+ static {
+ A.a = 10; // write other's static field, fail
+ b = A.a; // read other's static field, fail
+ }
+}
+
+class C {
+ public static int c;
+ static {
+ c = A.a; // read other's static field, fail
+ }
+}