Add support for sanitizers at compile time (and fix some issues) (#2599)

* Add an option to enable sanitizer at compile time * Fix singleton classes initialization * Revert r5378 - issue #1068 - to avoid having duplicate symbols exported by modules, that may cause a crash * Fix cast in KviTalIconAndRichTextItemDelegate (used by both QListWidget and QTreeWidget) * ScriptEditor: don't leak a timer for every script editor widget * Add support for memory and thread sanitizers * Fix sanitizer * KviSignalhandler: don't leak the whole object * Remove debug warning when a single message is longer than 512 bytes. As per https://ircv3.net/specs/extensions/message-tags.html#size-limit , message tags can prepend a whooping 8kbs of "tags" (header data) to each message. * Avoid initialization loop between KviLocale and KviMessageCatalogue. Assume all .po files are UTF8. This is already a requirement since about a decade. * KviInputEditor: don't leak the undo/redo stacks KviInputEditor: don't leak the undo/redo stacks - rework using smart pointers * Fix typo
kvirc · Feb 8, 2024 · 5cbbeab · 5cbbeab
1 parent b962ce4
commit 5cbbeab
Show file tree

Hide file tree

Showing 17 changed files with 52 additions and 124 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -187,6 +187,8 @@ if(CMAKE_BUILD_TYPE MATCHES Debug OR CMAKE_BUILD_TYPE MATCHES RelWithDebInfo OR
 endif()
 
 option(WANT_STRIP "Strip binaries (discard symbols from object files)" OFF)
+set(WANT_SANITIZER OFF CACHE STRING "Enable debug sanitizers (requires WANT_DEBUG)")
+set_property(CACHE WANT_SANITIZER PROPERTY STRINGS OFF ADDRESS MEMORY THREAD)
 
 if(WANT_ENV_FLAGS)
 	set(CMAKE_BUILD_TYPE Undefined)
@@ -233,6 +235,22 @@ else()
 				set(CMAKE_C_FLAGS "-O0 -g")
 			endif()
 		endif()
+		string(TOUPPER "${WANT_SANITIZER}" WANT_SANITIZER )
+		if(WANT_SANITIZER STREQUAL ADDRESS)
+			set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fsanitize=address -fno-omit-frame-pointer")
+			set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=undefined -fsanitize=address -fno-omit-frame-pointer")
+			set(CMAKE_STATUS_DEBUG_SUPPORT "Yes, with address,leak and undefined behavior sanitizers")
+		elseif(WANT_SANITIZER STREQUAL MEMORY)
+			set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=memory -fPIE -pie -fno-omit-frame-pointer")
+			set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=memory -fPIE -pie -fno-omit-frame-pointer")
+			set(CMAKE_STATUS_DEBUG_SUPPORT "Yes, with memory sanitizer")
+		elseif(WANT_SANITIZER STREQUAL THREAD)
+			set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=thread -fno-omit-frame-pointer")
+			set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=thread -fno-omit-frame-pointer")
+			set(CMAKE_STATUS_DEBUG_SUPPORT "Yes, with thread sanitizer")
+		elseif(NOT WANT_SANITIZER STREQUAL OFF)
+			message(FATAL_ERROR "Invalid sanitizer ${WANT_SANITIZER}. Available sanitizers: address, memory, thread")
+		endif()
 	else()
 		if(WANT_STRIP)
 			find_program(STRIP_EXECUTABLE NAMES strip)

diff --git a/src/kvilib/irc/KviIdentityProfileSet.cpp b/src/kvilib/irc/KviIdentityProfileSet.cpp
@@ -26,7 +26,6 @@
 #include "KviConfigurationFile.h"
 
 KviIdentityProfileSet * KviIdentityProfileSet::m_pSelf = nullptr;
-unsigned int KviIdentityProfileSet::m_uCount = 0;
 
 KviIdentityProfileSet::KviIdentityProfileSet()
     : KviHeapObject()
@@ -49,18 +48,18 @@ KviIdentityProfileSet::~KviIdentityProfileSet()
 
 void KviIdentityProfileSet::init()
 {
-	if((!m_pSelf) && (m_pSelf->count() == 0))
+	if(!m_pSelf)
 	{
 		m_pSelf = new KviIdentityProfileSet();
-		m_uCount++;
 	}
 }
 
 void KviIdentityProfileSet::done()
 {
-	m_uCount--;
-	if(m_pSelf->count() == 0)
+	if(m_pSelf) {
 		delete m_pSelf;
+		m_pSelf = nullptr;
+	}
 }
 
 void KviIdentityProfileSet::clear()

diff --git a/src/kvilib/irc/KviIdentityProfileSet.h b/src/kvilib/irc/KviIdentityProfileSet.h
@@ -66,7 +66,6 @@ class KVILIB_API KviIdentityProfileSet : public KviHeapObject
 
 private:
 	static KviIdentityProfileSet * m_pSelf;
-	static unsigned int m_uCount;
 
 protected:
 	KviPointerList<KviIdentityProfile> * m_pProfiles;
@@ -91,12 +90,6 @@ class KVILIB_API KviIdentityProfileSet : public KviHeapObject
 	*/
 	static inline KviIdentityProfileSet * instance() { return m_pSelf; };
 
-	/**
-	* \brief Returns the number of instances of the class
-	* \return unsigned int
-	*/
-	unsigned int count() { return m_uCount; };
-
 	/**
 	* \brief Returns the profiles set
 	* \return KviPointerList<KviIdentityProfile> *

diff --git a/src/kvilib/locale/KviLocale.cpp b/src/kvilib/locale/KviLocale.cpp
@@ -466,7 +466,6 @@ static KviLocale::EncodingDescription supported_encodings[] = {
 };
 
 KviLocale * KviLocale::m_pSelf = nullptr;
-unsigned int KviLocale::m_uCount = 0;
 QString KviLocale::g_szLang = "";
 
 KviLocale::KviLocale(QApplication * pApp, const QString & szLocaleDir, const QString & szForceLocaleDir)
@@ -559,18 +558,18 @@ KviLocale::~KviLocale()
 
 void KviLocale::init(QApplication * pApp, const QString & szLocaleDir, const QString & szForceLocaleDir)
 {
-	if((!m_pSelf) && (m_pSelf->count() == 0))
+	if(!m_pSelf)
 	{
 		m_pSelf = new KviLocale(pApp, szLocaleDir, szForceLocaleDir);
-		m_uCount++;
 	}
 }
 
 void KviLocale::done()
 {
-	m_uCount--;
-	if(m_pSelf->count() == 0)
+	if(m_pSelf) {
 		delete m_pSelf;
+		m_pSelf = nullptr;
+	}
 }
 
 QTextCodec * KviLocale::codecForName(const char * pcName)

diff --git a/src/kvilib/locale/KviLocale.h b/src/kvilib/locale/KviLocale.h
@@ -97,7 +97,6 @@ class KVILIB_API KviLocale : public KviHeapObject
 
 private:
 	static KviLocale * m_pSelf;
-	static unsigned int m_uCount;
 
 public:
 	/**
@@ -122,12 +121,6 @@ class KVILIB_API KviLocale : public KviHeapObject
 	*/
 	static inline KviLocale * instance() { return m_pSelf; }
 
-	/**
-	* \brief Returns the number of instances of the class
-	* \return unsigned int
-	*/
-	unsigned int count() { return m_uCount; }
-
 	/**
 	* \brief Returns the description of the encoding used
 	* \param iIdx The index of the description

diff --git a/src/kvilib/locale/KviMessageCatalogue.cpp b/src/kvilib/locale/KviMessageCatalogue.cpp
@@ -130,7 +130,7 @@ int kvi_getFirstBiggerPrime(int iNumber)
 KviMessageCatalogue::KviMessageCatalogue()
 {
 	//m_uEncoding = 0;
-	m_pTextCodec = QTextCodec::codecForLocale();
+	m_pTextCodec = QTextCodec::codecForName("UTF-8");
 
 	//m_pMessages = new KviPointerHashTable<const char *,KviTranslationEntry>(1123,true,false); // dictSize, case sensitive, don't copy keys
 	m_pMessages = new KviPointerHashTable<const char *, KviTranslationEntry>(32, true, false); // dictSize, case sensitive, don't copy keys
@@ -280,36 +280,6 @@ bool KviMessageCatalogue::load(const QString & szName)
 
 	KviMemory::free(pcBuffer);
 	f.close();
-
-	m_pTextCodec = nullptr;
-
-	// find out the text encoding, if possible
-	if(szHeader.hasData())
-	{
-		// find "charset=*\n"
-		int iIdx = szHeader.findFirstIdx("charset=");
-		if(iIdx != -1)
-		{
-			szHeader.cutLeft(iIdx + 8);
-			szHeader.cutFromFirst('\n');
-			szHeader.trim();
-			m_pTextCodec = KviLocale::instance()->codecForName(szHeader.ptr());
-			if(!m_pTextCodec)
-			{
-				qDebug("Can't find the codec for charset=%s", szHeader.ptr());
-				qDebug("Falling back to codecForLocale()");
-				m_pTextCodec = QTextCodec::codecForLocale();
-			}
-		}
-	}
-
-	if(!m_pTextCodec)
-	{
-		qDebug("The message catalogue does not have a \"charset\" header");
-		qDebug("Assuming UTF-8"); // FIXME: or codecForLocale() ?
-		m_pTextCodec = QTextCodec::codecForName("UTF-8");
-	}
-
 	return true;
 }
 

diff --git a/src/kvilib/system/KviSignalHandler.cpp b/src/kvilib/system/KviSignalHandler.cpp
@@ -64,7 +64,7 @@ KviSignalHandler::KviSignalHandler(QObject *parent)
 
 bool kvi_signalHandlerSetup()
 {
-	new KviSignalHandler();
+	new KviSignalHandler(qApp);
 
 	struct sigaction sa;
 	::memset(&sa,0,sizeof(sa));

diff --git a/src/kvilib/tal/KviTalIconAndRichTextItemDelegate.cpp b/src/kvilib/tal/KviTalIconAndRichTextItemDelegate.cpp
@@ -27,7 +27,6 @@
 #include <QAbstractItemView>
 #include <QAbstractTextDocumentLayout>
 #include <QApplication>
-#include <QListWidget>
 #include <QPainter>
 
 #define LVI_AFTER_ICON (LVI_BORDER + LVI_ICON_SIZE + LVI_SPACING)
@@ -102,15 +101,15 @@ QSize KviTalIconAndRichTextItemDelegate::sizeHint(const QStyleOptionViewItem & o
 	QTextDocument doc;
 	doc.setHtml(szText);
 	doc.setDefaultFont(option.font);
-	doc.setTextWidth(((QListWidget *)parent())->viewport()->width() - LVI_AFTER_ICON - LVI_BORDER);
+	doc.setTextWidth(((QAbstractItemView *)parent())->viewport()->width() - LVI_AFTER_ICON - LVI_BORDER);
 	int iHeight = doc.documentLayout()->documentSize().toSize().height();
 
-	//qDebug("Size hint (%d,%d)",((QListWidget *)parent())->minimumWidth(), iHeight + (2 * LVI_BORDER));
+	//qDebug("Size hint (%d,%d)",((QAbstractItemView *)parent())->minimumWidth(), iHeight + (2 * LVI_BORDER));
 
 	int iIconWidth = m_oIconSize.width() + (2 * LVI_BORDER);
 	int iIconHeight = m_oIconSize.height() + (2 * LVI_BORDER);
 
-	int w = ((QListWidget *)parent())->minimumWidth();
+	int w = ((QAbstractItemView *)parent())->minimumWidth();
 	if(w < iIconWidth)
 		w = iIconWidth;
 	if(w < m_oMinimumSize.width())

diff --git a/src/kvirc/kernel/KviDefaultScript.cpp b/src/kvirc/kernel/KviDefaultScript.cpp
@@ -41,7 +41,6 @@
 #include <QMessageBox>
 
 KviDefaultScriptManager * KviDefaultScriptManager::m_pSelf = nullptr;
-unsigned int KviDefaultScriptManager::m_uCount = 0;
 
 KviDefaultScriptManager::KviDefaultScriptManager()
     : QObject()
@@ -72,18 +71,18 @@ KviDefaultScriptManager::~KviDefaultScriptManager()
 
 void KviDefaultScriptManager::init()
 {
-	if((!m_pSelf) && (m_pSelf->count() == 0))
+	if(!m_pSelf)
 	{
 		m_pSelf = new KviDefaultScriptManager();
-		m_uCount++;
 	}
 }
 
 void KviDefaultScriptManager::done()
 {
-	m_uCount--;
-	if(m_pSelf->count() == 0)
+	if(m_pSelf) {
 		delete m_pSelf;
+		m_pSelf = nullptr;
+	}
 }
 
 bool KviDefaultScriptManager::isDefscriptUpToDate()

diff --git a/src/kvirc/kernel/KviDefaultScript.h b/src/kvirc/kernel/KviDefaultScript.h
@@ -61,7 +61,6 @@ class KVIRC_API KviDefaultScriptManager : public QObject
 
 private:
 	static KviDefaultScriptManager * m_pSelf;
-	static unsigned int m_uCount;
 	bool m_bNoNeedToRestore = false;
 	bool m_bConfigFileMissing = false;
 	KviDefaultScriptDialog * m_pDialog = nullptr;
@@ -95,12 +94,6 @@ class KVIRC_API KviDefaultScriptManager : public QObject
 	*/
 	static inline KviDefaultScriptManager * instance() { return m_pSelf; }
 
-	/**
-	* \brief Returns the number of instances of the class
-	* \return unsigned int
-	*/
-	unsigned int count() const { return m_uCount; }
-
 	/**
 	* \brief Checks if the local defscript is up to date
 	* \return bool

diff --git a/src/kvirc/kernel/KviIrcLink.cpp b/src/kvirc/kernel/KviIrcLink.cpp
@@ -296,11 +296,6 @@ void KviIrcLink::processData(char * buffer, int iLen)
 			m_pReadBuffer = (char *)KviMemory::allocate(m_uReadBufferLen);
 			KviMemory::move(m_pReadBuffer, cBeginOfCurData, m_uReadBufferLen);
 		}
-		//The m_pReadBuffer contains at max 1 IRC message...
-		//that can not be longer than 510 bytes (the message is not CRLF terminated)
-		// FIXME: Is this limit *really* valid on all servers ?
-		if(m_uReadBufferLen > 510)
-			qDebug("WARNING: receiving an invalid IRC message from server.");
 	}
 	KviMemory::free(cMessageBuffer);
 }

diff --git a/src/kvirc/module/KviModuleManager.cpp b/src/kvirc/module/KviModuleManager.cpp
@@ -175,8 +175,6 @@ bool KviModuleManager::loadModule(const QString & modName)
 	}
 
 	QLibrary * pLibrary = new QLibrary(tmp);
-	pLibrary->setLoadHints(QLibrary::ExportExternalSymbolsHint);
-
 	if(!pLibrary->load())
 	{
 		m_szLastError = pLibrary->errorString();

diff --git a/src/kvirc/ui/KviInputEditor.cpp b/src/kvirc/ui/KviInputEditor.cpp
@@ -216,6 +216,7 @@ KviInputEditor::~KviInputEditor()
 	if(m_iCursorTimer)
 		killTimer(m_iCursorTimer);
 	killDragTimer();
+	clearUndoStack();
 
 	qDeleteAll(m_p->lTextBlocks);
 	delete m_p;
@@ -2602,7 +2603,7 @@ void KviInputEditor::undo()
 	if(m_UndoStack.empty())
 		return; // this should be ensured by isUndoAvailable() but well...
 
-	EditCommand * pCommand = m_UndoStack.back();
+	std::unique_ptr<EditCommand> pCommand = std::move(m_RedoStack.back());
 	m_UndoStack.pop_back();
 
 	Q_ASSERT(pCommand); // should be true: we delete the empty undo stack
@@ -2624,12 +2625,10 @@ void KviInputEditor::undo()
 			break;
 		default:
 			Q_ASSERT_X(false, "KviInputEditor::undo", "Unexpected EditCommand type");
-			delete pCommand; // argh
 			return;
-			break;
 	}
 
-	m_RedoStack.push_back(pCommand);
+	m_RedoStack.push_back(std::move(pCommand));
 	if(m_RedoStack.size() > KVI_INPUT_MAX_UNDO_SIZE)
 		m_RedoStack.erase(m_RedoStack.begin()); // will delete it
 }
@@ -2642,7 +2641,7 @@ void KviInputEditor::redo()
 	if(m_RedoStack.empty())
 		return; // this should be ensured by isUndoAvailable() but well...
 
-	EditCommand * pCommand = m_RedoStack.back();
+	std::unique_ptr<EditCommand> pCommand = std::move(m_RedoStack.back());
 	m_RedoStack.pop_back();
 
 	Q_ASSERT(pCommand); // should be true: we delete the empty redo stack
@@ -2664,19 +2663,17 @@ void KviInputEditor::redo()
 			break;
 		default:
 			Q_ASSERT_X(false, "KviInputEditor::redo", "Unexpected EditCommand type");
-			delete pCommand; // argh
 			return;
-			break;
 	}
 
-	m_UndoStack.push_back(pCommand);
+	m_UndoStack.push_back(std::move(pCommand));
 	if(m_UndoStack.size() > KVI_INPUT_MAX_UNDO_SIZE)
 		m_UndoStack.erase(m_UndoStack.begin()); // will delete it
 }
 
 void KviInputEditor::addUndo(EditCommand * pCommand)
 {
-	m_UndoStack.push_back(pCommand);
+	m_UndoStack.push_back(std::unique_ptr<EditCommand>(pCommand));
 
 	if(m_UndoStack.size() > KVI_INPUT_MAX_UNDO_SIZE)
 		m_UndoStack.erase(m_UndoStack.begin()); // will delete it

diff --git a/src/kvirc/ui/KviInputEditor.h b/src/kvirc/ui/KviInputEditor.h
@@ -253,7 +253,7 @@ class KVIRC_API KviInputEditor : public QWidget
 	* Contains owned pointers and has autodelete set to true. The most recent command
 	* is at the end. Null when no undo is available.
 	*/
-	std::vector<EditCommand *> m_UndoStack;
+	std::vector<std::unique_ptr<EditCommand>> m_UndoStack;
 
 	/**
 	* \var m_RedoStack
@@ -262,7 +262,7 @@ class KVIRC_API KviInputEditor : public QWidget
 	* Contains owned pointers and has autodelete set to true. The most recently undone
 	* command is at the end. Null when no redo is available.
 	*/
-	std::vector<EditCommand *> m_RedoStack;
+	std::vector<std::unique_ptr<EditCommand>> m_RedoStack;
 
 	KviInputEditorPrivate * m_p;