Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disable decayment of freed Objects to null in debug builds #41866

Merged
merged 1 commit into from
Oct 1, 2020
Merged

Disable decayment of freed Objects to null in debug builds #41866

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/variant.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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