Change window allocation/destruction to avoid undefined behaviour

Create a new window base class which holds the front/back pointers
and the window class.
This fixes the voluminous warning spam about deleted windows when using
UndefinedBehaviorSanitizer.

src/viewport.cpp

@@ -480,7 +480,7 @@ static void SetViewportPosition(Window *w, int x, int y, bool force_update_overl
 		i = top + height - _screen.height;
 		if (i >= 0) height -= i;
 
-		if (height > 0) DoSetViewportPosition(w->z_front, left, top, width, height);
+		if (height > 0) DoSetViewportPosition((const Window *) w->z_front, left, top, width, height);
 	}
 }
 

src/window.cpp

@@ -52,9 +52,9 @@ static Window *_mouseover_last_w = NULL; ///< Window of the last #MOUSEOVER even
 static Window *_last_scroll_window = NULL; ///< Window of the last scroll event.
 
 /** List of windows opened at the screen sorted from the front. */
-Window *_z_front_window = NULL;
+WindowBase *_z_front_window = NULL;
 /** List of windows opened at the screen sorted from the back. */
-Window *_z_back_window  = NULL;
+WindowBase *_z_back_window  = NULL;
 
 /** If false, highlight is white, otherwise the by the widget defined colour. */
 bool _window_highlight_colour = false;
@@ -1359,7 +1359,7 @@ static void AddWindowToZOrdering(Window *w)
 		w->z_front = w->z_back = NULL;
 	} else {
 		/* Search down the z-ordering for its location. */
-		Window *v = _z_front_window;
+		WindowBase *v = _z_front_window;
 		uint last_z_priority = UINT_MAX;
 		while (v != NULL && (v->window_class == WC_INVALID || GetWindowZPriority(v->window_class) > GetWindowZPriority(w->window_class))) {
 			if (v->window_class != WC_INVALID) {
@@ -1398,7 +1398,7 @@ static void AddWindowToZOrdering(Window *w)
  * Removes a window from the z-ordering.
  * @param w Window to remove
  */
-static void RemoveWindowFromZOrdering(Window *w)
+static void RemoveWindowFromZOrdering(WindowBase *w)
 {
 	if (w->z_front == NULL) {
 		assert(_z_front_window == w);
@@ -1897,11 +1897,11 @@ void UnInitWindowSystem()
 {
 	UnshowCriticalError();
 
-	Window *w;
+	Window *v;
-	FOR_ALL_WINDOWS_FROM_FRONT(w) delete w;
+	FOR_ALL_WINDOWS_FROM_FRONT(v) delete v;
 
-	for (w = _z_front_window; w != NULL; /* nothing */) {
+	for (WindowBase *w = _z_front_window; w != NULL; /* nothing */) {
-		Window *to_del = w;
+		WindowBase *to_del = w;
 		w = w->z_back;
 		free(to_del);
 	}
@@ -3080,8 +3080,8 @@ void InputLoop()
 	HandleKeyScrolling();
 
 	/* Do the actual free of the deleted windows. */
-	for (Window *v = _z_front_window; v != NULL; /* nothing */) {
+	for (WindowBase *v = _z_front_window; v != NULL; /* nothing */) {
-		Window *w = v;
+		WindowBase *w = v;
 		v = v->z_back;
 
 		if (w->window_class != WC_INVALID) continue;

src/window_gui.h

@@ -145,8 +145,8 @@ void DrawFrameRect(int left, int top, int right, int bottom, Colours colour, Fra
 void DrawCaption(const Rect &r, Colours colour, Owner owner, StringID str);
 
 /* window.cpp */
-extern Window *_z_front_window;
+extern WindowBase *_z_front_window;
-extern Window *_z_back_window;
+extern WindowBase *_z_back_window;
 extern Window *_focused_window;
 
 inline uint64 GetWindowUpdateNumber()
@@ -277,10 +277,48 @@ struct ViewportData : ViewPort {
 
 struct QueryString;
 
+struct WindowBase {
+	WindowBase *z_front;             ///< The window in front of us in z-order.
+	WindowBase *z_back;              ///< The window behind us in z-order.
+	WindowClass window_class;        ///< Window class
+
+	virtual ~WindowBase() {}
+
+	/**
+	 * Memory allocator for a single class instance.
+	 * @param size the amount of bytes to allocate.
+	 * @return the given amounts of bytes zeroed.
+	 */
+	inline void *operator new(size_t size) { return CallocT<byte>(size); }
+
+protected:
+	WindowBase() {}
+
+private:
+	/**
+	 * Memory allocator for an array of class instances.
+	 * @param size the amount of bytes to allocate.
+	 * @return the given amounts of bytes zeroed.
+	 */
+	inline void *operator new[](size_t size) { NOT_REACHED(); }
+
+	/**
+	 * Memory release for a single class instance.
+	 * @param ptr  the memory to free.
+	 */
+	inline void operator delete(void *ptr) { NOT_REACHED(); }
+
+	/**
+	 * Memory release for an array of class instances.
+	 * @param ptr  the memory to free.
+	 */
+	inline void operator delete[](void *ptr) { NOT_REACHED(); }
+};
+
 /**
  * Data structure for an opened window
  */
-struct Window : ZeroedMemoryAllocator {
+struct Window : WindowBase {
 protected:
 	void InitializeData(WindowNumber window_number);
 	void InitializePositionSize(int x, int y, int min_width, int min_height);
@@ -315,7 +353,6 @@ public:
 
 	WindowDesc *window_desc;    ///< Window description
 	WindowFlags flags;          ///< Window flags
-	WindowClass window_class;   ///< Window class
 	WindowNumber window_number; ///< Window number within the window class
 
 	uint8 timeout_timer;      ///< Timer value of the WF_TIMEOUT for flags.
@@ -342,8 +379,6 @@ public:
 	int scrolling_scrollbar;         ///< Widgetindex of just being dragged scrollbar. -1 if none is active.
 
 	Window *parent;                  ///< Parent window.
-	Window *z_front;                 ///< The window in front of us in z-order.
-	Window *z_back;                  ///< The window behind us in z-order.
 
 	template <class NWID>
 	inline const NWID *GetWidget(uint widnum) const;
@@ -891,9 +926,37 @@ void GuiShowTooltips(Window *parent, StringID str, uint paramcount = 0, const ui
 /* widget.cpp */
 int GetWidgetFromPos(const Window *w, int x, int y);
 
+inline const Window *FromBaseWindowFront(const WindowBase *w)
+{
+	while (w) {
+		if (w->window_class != WC_INVALID) return (const Window *) w;
+		w = w->z_front;
+	}
+	return nullptr;
+}
+
+inline Window *FromBaseWindowFront(WindowBase *w)
+{
+	return const_cast<Window *>(FromBaseWindowFront(const_cast<const WindowBase *>(w)));
+}
+
+inline const Window *FromBaseWindowBack(const WindowBase *w)
+{
+	while (w) {
+		if (w->window_class != WC_INVALID) return (const Window *) w;
+		w = w->z_back;
+	}
+	return nullptr;
+}
+
+inline Window *FromBaseWindowBack(WindowBase *w)
+{
+	return const_cast<Window *>(FromBaseWindowBack(const_cast<const WindowBase *>(w)));
+}
+
 /** Iterate over all windows */
-#define FOR_ALL_WINDOWS_FROM_BACK_FROM(w, start)  for (w = start; w != NULL; w = w->z_front) if (w->window_class != WC_INVALID)
+#define FOR_ALL_WINDOWS_FROM_BACK_FROM(w, start)  for (w = FromBaseWindowFront(start); w != NULL; w = FromBaseWindowFront(w->z_front))
-#define FOR_ALL_WINDOWS_FROM_FRONT_FROM(w, start) for (w = start; w != NULL; w = w->z_back) if (w->window_class != WC_INVALID)
+#define FOR_ALL_WINDOWS_FROM_FRONT_FROM(w, start) for (w = FromBaseWindowBack(start); w != NULL; w = FromBaseWindowBack(w->z_back))
 #define FOR_ALL_WINDOWS_FROM_BACK(w)  FOR_ALL_WINDOWS_FROM_BACK_FROM(w, _z_back_window)
 #define FOR_ALL_WINDOWS_FROM_FRONT(w) FOR_ALL_WINDOWS_FROM_FRONT_FROM(w, _z_front_window)
 

src/window_type.h

@@ -755,6 +755,7 @@ enum GameOptionsInvalidationData {
 };
 
 struct Window;
+struct WindowBase;
 
 /** Number to differentiate different windows of the same class */
 typedef int32 WindowNumber;