clooneljump/docs/wrong_virtual_call.md

30 lines
1.9 KiB
Markdown

## Bug description ##
1) SizeNotifiable, defined as the following:
class SizeNotifiable<RegisterBehaviour, true> : private RegisterBehaviour, public SizeNotifiableBase { ... };
gets constructed - it inherits from SizeNotifiableBase which declares NotifyResChanged() as virtual. It also inherits from RegisterBehaviour, which must provide a Register() method.
2) In SizeNotifiable's constructor RegisterBehaviour::Register(this) gets called. Being in the constructor, virtual calls on this are UB.
3) When RegisterBehaviour == AutoRegister, we have Register defined as the following:
void AutoRegister::Register (SizeNotifiableBase* parNotifiable) {
m_id = m_sdlmain->RegisterForResChange(parNotifiable);
}
note that parNotifiable is received from SizeNotifiable's constructor, so it's a partially constructed object.
4) RegisterForResChange() is defined as:
size_t SDLMain::RegisterForResChange (SizeNotifiableBase* parNotif) {
parNotif->NotifyResChanged(m_localData->sizeratio);
return m_localData->resChangeNotifList.Add(parNotif);
}
parNotif is, again, the partly constructed object pointed by this in SizeNotifiable's constructor. NotifyResChanged() is a virtual method, thus the bug.
Objects being registered need to be notified straight away since they have no clue about the current size held by SDLMain.
## Solution ##
Fixed at commit 5e76b49cae954accd1533d6b6431af7cb5f770ef doing something that I call *deferred virtual call*. A temporary object is passed down in the SizeNotifiable's hierarchy. This object's destructor calls the Register() method *if the stack is not being unwinded due to an exception*. Since the object is passed from the top (from outside the topmost constructor), its destructor gets called after the end of the scope of the topmost constructor in the SizeNotifiable's hierarchy. By that point all virtual tables should be set to their final state.