Kaynağa Gözat

Merge pull request #41866 from RandomShaper/fix_41179

Disable decayment of freed Objects to null in debug builds
Rémi Verschelde 5 yıl önce
ebeveyn
işleme
df2dcf5742
3 değiştirilmiş dosya ile 24 ekleme ve 14 silme
  1. 2 2
      core/variant.cpp
  2. 12 2
      core/variant.h
  3. 10 10
      core/variant_op.cpp

+ 2 - 2
core/variant.cpp

@@ -791,7 +791,7 @@ bool Variant::is_zero() const {
 		} break;
 		case OBJECT: {
 
-			return _OBJ_PTR(*this) == NULL;
+			return _UNSAFE_OBJ_PROXY_PTR(*this) == NULL;
 		} break;
 		case NODE_PATH: {
 
@@ -2865,7 +2865,7 @@ uint32_t Variant::hash() const {
 		} break;
 		case OBJECT: {
 
-			return hash_djb2_one_64(make_uint64_t(_OBJ_PTR(*this)));
+			return hash_djb2_one_64(make_uint64_t(_UNSAFE_OBJ_PROXY_PTR(*this)));
 		} break;
 		case NODE_PATH: {
 

+ 12 - 2
core/variant.h

@@ -73,11 +73,21 @@ typedef PoolVector<Color> PoolColorArray;
 #define GCC_ALIGNED_8
 #endif
 
+// With DEBUG_ENABLED, the pointer to a deleted object stored in ObjectRC is set to nullptr,
+// so _OBJ_PTR is not useful for checks in which we want to act as if we still believed the
+// object is alive; e.g., comparing a Variant that points to a deleted object with NIL,
+// should return false regardless DEBUG_ENABLED is defined or not.
+// So in cases like that we use _UNSAFE_OBJ_PROXY_PTR, which serves that purpose. With DEBUG_ENABLED
+// it won't be the real pointer to the object for non-Reference types, but that's fine.
+// We just need it to be unique for each object, to be comparable and not to be forced to NULL
+// when the object is freed.
 #ifdef DEBUG_ENABLED
-// Ideally, an inline member of ObjectRC, but would cause circular includes
-#define _OBJ_PTR(m_variant) ((m_variant)._get_obj().rc ? (m_variant)._get_obj().rc->get_ptr() : reinterpret_cast<Ref<Reference> *>((m_variant)._get_obj().ref.get_data())->ptr())
+#define _REF_OBJ_PTR(m_variant) (reinterpret_cast<Ref<Reference> *>((m_variant)._get_obj().ref.get_data())->ptr())
+#define _OBJ_PTR(m_variant) ((m_variant)._get_obj().rc ? (m_variant)._get_obj().rc->get_ptr() : _REF_OBJ_PTR(m_variant))
+#define _UNSAFE_OBJ_PROXY_PTR(m_variant) ((m_variant)._get_obj().rc ? reinterpret_cast<uint8_t *>((m_variant)._get_obj().rc) : reinterpret_cast<uint8_t *>(_REF_OBJ_PTR(m_variant)))
 #else
 #define _OBJ_PTR(m_variant) ((m_variant)._get_obj().obj)
+#define _UNSAFE_OBJ_PROXY_PTR(m_variant) _OBJ_PTR(m_variant)
 #endif
 
 class Variant {

+ 10 - 10
core/variant_op.cpp

@@ -400,7 +400,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_EQUAL, NIL) {
 				if (p_b.type == NIL) _RETURN(true);
 				if (p_b.type == OBJECT)
-					_RETURN(_OBJ_PTR(p_b) == NULL);
+					_RETURN(_UNSAFE_OBJ_PROXY_PTR(p_b) == NULL);
 
 				_RETURN(false);
 			}
@@ -417,9 +417,9 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 
 			CASE_TYPE(math, OP_EQUAL, OBJECT) {
 				if (p_b.type == OBJECT)
-					_RETURN(_OBJ_PTR(p_a) == _OBJ_PTR(p_b));
+					_RETURN(_UNSAFE_OBJ_PROXY_PTR(p_a) == _UNSAFE_OBJ_PROXY_PTR(p_b));
 				if (p_b.type == NIL)
-					_RETURN(_OBJ_PTR(p_a) == NULL);
+					_RETURN(_UNSAFE_OBJ_PROXY_PTR(p_a) == NULL);
 
 				_RETURN_FAIL;
 			}
@@ -487,7 +487,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_NOT_EQUAL, NIL) {
 				if (p_b.type == NIL) _RETURN(false);
 				if (p_b.type == OBJECT)
-					_RETURN(_OBJ_PTR(p_b) != NULL);
+					_RETURN(_UNSAFE_OBJ_PROXY_PTR(p_b) != NULL);
 
 				_RETURN(true);
 			}
@@ -505,9 +505,9 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 
 			CASE_TYPE(math, OP_NOT_EQUAL, OBJECT) {
 				if (p_b.type == OBJECT)
-					_RETURN((_OBJ_PTR(p_a) != _OBJ_PTR(p_b)));
+					_RETURN((_UNSAFE_OBJ_PROXY_PTR(p_a) != _UNSAFE_OBJ_PROXY_PTR(p_b)));
 				if (p_b.type == NIL)
-					_RETURN(_OBJ_PTR(p_a) != NULL);
+					_RETURN(_UNSAFE_OBJ_PROXY_PTR(p_a) != NULL);
 
 				_RETURN_FAIL;
 			}
@@ -590,7 +590,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_LESS, OBJECT) {
 				if (p_b.type != OBJECT)
 					_RETURN_FAIL;
-				_RETURN(_OBJ_PTR(p_a) < _OBJ_PTR(p_b));
+				_RETURN(_UNSAFE_OBJ_PROXY_PTR(p_a) < _UNSAFE_OBJ_PROXY_PTR(p_b));
 			}
 
 			CASE_TYPE(math, OP_LESS, ARRAY) {
@@ -644,7 +644,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_LESS_EQUAL, OBJECT) {
 				if (p_b.type != OBJECT)
 					_RETURN_FAIL;
-				_RETURN(_OBJ_PTR(p_a) <= _OBJ_PTR(p_b));
+				_RETURN(_UNSAFE_OBJ_PROXY_PTR(p_a) <= _UNSAFE_OBJ_PROXY_PTR(p_b));
 			}
 
 			DEFAULT_OP_NUM(math, OP_LESS_EQUAL, INT, <=, _int);
@@ -694,7 +694,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_GREATER, OBJECT) {
 				if (p_b.type != OBJECT)
 					_RETURN_FAIL;
-				_RETURN(_OBJ_PTR(p_a) > _OBJ_PTR(p_b));
+				_RETURN(_UNSAFE_OBJ_PROXY_PTR(p_a) > _UNSAFE_OBJ_PROXY_PTR(p_b));
 			}
 
 			CASE_TYPE(math, OP_GREATER, ARRAY) {
@@ -748,7 +748,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_GREATER_EQUAL, OBJECT) {
 				if (p_b.type != OBJECT)
 					_RETURN_FAIL;
-				_RETURN(_OBJ_PTR(p_a) >= _OBJ_PTR(p_b));
+				_RETURN(_UNSAFE_OBJ_PROXY_PTR(p_a) >= _UNSAFE_OBJ_PROXY_PTR(p_b));
 			}
 
 			DEFAULT_OP_NUM(math, OP_GREATER_EQUAL, INT, >=, _int);