-
Notifications
You must be signed in to change notification settings - Fork 39
Description
Use of ObjPtr<T> should be replaced with T*, and ObjRefPtr<T> should be replaced with T** to avoid subtle bugs.
In #565, &GetSwapchain()->GetPresentationReadySemaphore(imageIndex)) is bad since I'm taking the address-of an rvalue. This leads to stack-use-after-scope: at the end of the line, the SemaphorePtr (ObjPtr<Semaphore>) returned by GetPresentationReadySemaphore goes out of scope; however, I captured a reference to the value inside the SemaphorePtr (via the address-of operator) which is now invalid.
This was allowed since ObjPtr<Semaphore> overloads the address-of operator (operator&). Normally, &ObjPtr<T> would give me a ObjPtr<T>*. However, given the overload, I get the T* contained inside the class. The compiler didn't stop me from doing so. In normal cases, however, it would. Consider:
int i = 0;
int** i_ptr = &(&i);
// Clang: error: cannot take the address of an rvalue of type 'int *
// gcc: error: lvalue required as unary '&' operand
// This makes sense because &i doesn't have a memory address yet.
// How can we have an address of something that doesn't have an address?However, here, it doesn't:
// Pared-down version of ObjPtrRef
template <typename T>
class PtrRef {
public:
PtrRef(T* ref) : ref(ref) {};
operator T**() {
return ref;
}
T** ref;
};
// Pared-down version of ObjPtr
template <typename T>
class Ptr {
public:
Ptr(T* me) : me(me) {};
T** operator&() { // DANGER!
return &me;
}
T* me;
};
int i = 0;
int** i_ptr = &Ptr<int>(&i);
// no error but incorrect behavior
// by now, Ptr<int> has been destroyed.
// That means Ptr<int>.me is invalid, as is ref = &me Note that Google Style says:
Do not overload &&, ||, , (comma), or unary &.
- https://google.github.io/styleguide/cppguide.html#Operator_Overloading
There are a variety of options, each more involved than the last:
- New functions should prefer returning T* or T** instead of ObjPtr or ObjPtrRef
- Remove
operator&from ObjPtr in favor of an explicit.get()function that returns T*. This is what smart pointers (unique_ptr,shared_ptr) do. Since they return a T*, the compiler will catch mistakes. - Remove ObjPtr entirely. This might be easier than expected since mostly the aliases are used rather than ObjPtr itself.