Skip to content

Commit

Permalink
Disable decayment of freed objects to null in debug builds
Browse files Browse the repository at this point in the history
  • Loading branch information
@RandomShaper
RandomShaper committed Sep 8, 2020
1 parent 939ed5d commit ddd8691
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 14 deletions.
4 changes: 2 additions & 2 deletions core/variant.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {

Expand Down Expand Up @@ -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: {

Expand Down
14 changes: 12 additions & 2 deletions core/variant.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 10 additions & 10 deletions core/variant_op.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit ddd8691

Please sign in to comment.