Remove indenting APIs from Logger. These are thread unsafe by design.

Loggers are not owned by a specific thread and many are used by multiple threads. The indenting APIs could not be made to be thread safe in any way as they exist, so they have been removed. The data races caused by the indenting APIs are actually an important stability issue that could cause the indentation level to go out of bounds and crash the product.

If you were using the indentation APIs, you will need to rewrite the usage to manually manage the indentation level in a thread safe way.

4 files changed, 49 insertions, 120 deletions

diff --git a/binaryninjaapi.h b/binaryninjaapi.h
index b8dde5cd..d3de5874 100644
--- a/binaryninjaapi.h
+++ b/binaryninjaapi.h
@@ -1870,24 +1870,6 @@ namespace BinaryNinja {
 				\return The logger session ID
 			*/
 			size_t GetSessionId();
-
-			/*! Indent the logger's indentation level by one
-			 */
-			void Indent();
-
-			/*! Decrease the logger's indentation level by one
-			 */
-			void Dedent();
-
-			/*! Set the logger's indentation level to zero
-			 */
-			void ResetIndent();
-
-			/*! Get the string to prepend to log messages to indent them
-
-				\return Indentation string
-			 */
-			std::string GetIndent() const;
 	};
 
 	/*! A class allowing registering and retrieving Loggers
@@ -1948,24 +1930,6 @@ namespace BinaryNinja {
 		static std::vector<std::string> GetLoggerNames();
 	};
 
-	/*! RAII helper that indents/dedents a Logger inside a scope
-		\ingroup logging
-	 */
-	class LoggerIndentScope
-	{
-		Ref<Logger> m_logger;
-
-	public:
-		LoggerIndentScope(Ref<Logger> logger): m_logger(logger)
-		{
-			m_logger->Indent();
-		}
-		~LoggerIndentScope()
-		{
-			m_logger->Dedent();
-		}
-	};
-
 	/*!
 		@addtogroup coreapi
 	 	@{
diff --git a/binaryninjacore.h b/binaryninjacore.h
index 0ba01542..305fc713 100644
--- a/binaryninjacore.h
+++ b/binaryninjacore.h
@@ -37,14 +37,14 @@
 // Current ABI version for linking to the core. This is incremented any time
 // there are changes to the API that affect linking, including new functions,
 // new types, or modifications to existing functions or types.
-#define BN_CURRENT_CORE_ABI_VERSION 145
+#define BN_CURRENT_CORE_ABI_VERSION 146
 
 // Minimum ABI version that is supported for loading of plugins. Plugins that
 // are linked to an ABI version less than this will not be able to load and
 // will require rebuilding. The minimum version is increased when there are
 // incompatible changes that break binary compatibility, such as changes to
 // existing types or functions.
-#define BN_MINIMUM_CORE_ABI_VERSION 141
+#define BN_MINIMUM_CORE_ABI_VERSION 146
 
 #ifdef __GNUC__
 	#ifdef BINARYNINJACORE_LIBRARY
@@ -4031,10 +4031,6 @@ extern "C"
 
 	BINARYNINJACOREAPI char* BNLoggerGetName(BNLogger* logger);
 	BINARYNINJACOREAPI size_t BNLoggerGetSessionId(BNLogger* logger);
-	BINARYNINJACOREAPI void BNLoggerIndent(BNLogger* logger);
-	BINARYNINJACOREAPI void BNLoggerDedent(BNLogger* logger);
-	BINARYNINJACOREAPI void BNLoggerResetIndent(BNLogger* logger);
-	BINARYNINJACOREAPI char* BNGetLoggerIndent(BNLogger* logger);
 	BINARYNINJACOREAPI BNLogger* BNLogCreateLogger(const char* loggerName, size_t sessionId);
 	BINARYNINJACOREAPI BNLogger* BNLogGetLogger(const char* loggerName, size_t sessionId);
 	BINARYNINJACOREAPI char** BNLogGetLoggerNames(size_t* count);
diff --git a/demangler/msvc/demangle_msvc.cpp b/demangler/msvc/demangle_msvc.cpp
index c82d816a..cabee544 100644
--- a/demangler/msvc/demangle_msvc.cpp
+++ b/demangler/msvc/demangle_msvc.cpp
@@ -162,7 +162,7 @@ Demangle::Demangle(Architecture* arch, string mangledName) :
 	m_view(nullptr)
 {
 	m_logger = LogRegistry::CreateLogger("MSVCDemangle");
-	m_logger->ResetIndent();
+	//m_logger->ResetIndent();
 }
 
 
@@ -173,7 +173,7 @@ Demangle::Demangle(Ref<Platform> platform, string mangledName) :
 	m_view(nullptr)
 {
 	m_logger = LogRegistry::CreateLogger("MSVCDemangle");
-	m_logger->ResetIndent();
+	//m_logger->ResetIndent();
 }
 
 
@@ -186,7 +186,7 @@ Demangle::Demangle(Ref<BinaryView> view, string mangledName) :
 		throw DemangleException();
 	m_arch = m_platform->GetArchitecture();
 	m_logger = LogRegistry::CreateLogger("MSVCDemangle");
-	m_logger->ResetIndent();
+	//m_logger->ResetIndent();
 }
 
 
@@ -293,9 +293,9 @@ TypeBuilder Demangle::DemangleVarType(BackrefList& varList, bool isReturn, Quali
 		case 'O':
 		{
 			QualifiedName name;
-			m_logger->Indent();
+			//m_logger->Indent();
 			auto childType = DemangleVarType(varList, false, name);
-			m_logger->Dedent();
+			//m_logger->Dedent();
 			newType = TypeBuilder::ArrayType(childType.Finalize(), 0);
 			break;
 		}
@@ -340,9 +340,9 @@ TypeBuilder Demangle::DemangleVarType(BackrefList& varList, bool isReturn, Quali
 			reader.Consume(2);
 			DemangleModifiers(_const, _volatile, isMember);
 			QualifiedName name;
-			m_logger->Indent();
+			//m_logger->Indent();
 			newType = DemangleVarType(varList, false, name);
-			m_logger->Dedent();
+			//m_logger->Dedent();
 			newType.SetConst(_const);
 			newType.SetVolatile(_volatile);
 			return newType;
@@ -467,9 +467,9 @@ TypeBuilder Demangle::DemangleVarType(BackrefList& varList, bool isReturn, Quali
 					elementList.push_back(element);
 				}
 				QualifiedName name;
-				m_logger->Indent();
+				//m_logger->Indent();
 				child = DemangleVarType(varList, false, name);
-				m_logger->Dedent();
+				//m_logger->Dedent();
 
 				for (auto i = elementList.rbegin(); i != elementList.rend(); i++)
 				{
@@ -479,9 +479,9 @@ TypeBuilder Demangle::DemangleVarType(BackrefList& varList, bool isReturn, Quali
 			else
 			{
 				QualifiedName name;
-				m_logger->Indent();
+				//m_logger->Indent();
 				child = DemangleVarType(varList, true, name);
-				m_logger->Dedent();
+				//m_logger->Dedent();
 			}
 
 			child.SetConst(_const2);
@@ -501,17 +501,17 @@ TypeBuilder Demangle::DemangleVarType(BackrefList& varList, bool isReturn, Quali
 	}
 	case EnumerationTypeClass:
 		m_logger->LogDebug("Demangle enumeration\n");
-		m_logger->Indent();
+		//m_logger->Indent();
 		DemangleName(typeName, classFunctionType, varList);
-		m_logger->Dedent();
+		//m_logger->Dedent();
 		newType = TypeBuilder::NamedType(NamedTypeReference::GenerateAutoDemangledTypeReference(EnumNamedTypeClass, typeName),
 		                                 width, width);
 		break;
 	case StructureTypeClass:
 		m_logger->LogDebug("Demangle structure\n");
-		m_logger->Indent();
+		//m_logger->Indent();
 		DemangleName(typeName, classFunctionType, varList);
-		m_logger->Dedent();
+		//m_logger->Dedent();
 		switch (structType)
 		{
 		case ClassStructureType:
@@ -721,9 +721,9 @@ void Demangle::DemangleVariableList(vector<FunctionParameter>& paramList, Backre
 		FunctionParameter vt;
 		QualifiedName name;
 		m_logger->LogDebug("Argument %d: %s", i, reader.GetRaw());
-		m_logger->Indent();
+		//m_logger->Indent();
 		TypeBuilder type = DemangleVarType(varList, false, name);
-		m_logger->Dedent();
+		//m_logger->Dedent();
 		if (hasModifiers)
 		{
 			type.SetConst(_const);
@@ -820,9 +820,9 @@ void Demangle::DemangleNameTypeRtti(BNNameType& classFunctionType,
 		DemangleModifiers(_const, _volatile, isMember);
 
 		QualifiedName name;
-		m_logger->Indent();
+		//m_logger->Indent();
 		rtti = DemangleVarType(nameBackrefList, false, name);
-		m_logger->Dedent();
+		//m_logger->Dedent();
 		rtti.SetConst(_const);
 		rtti.SetVolatile(_volatile);
 		rtti.SetPointerSuffix(suffix);
@@ -991,9 +991,9 @@ string Demangle::DemangleTemplateInstantiationName(BackrefList& nameBackrefList)
 
 string Demangle::DemangleTemplateParams(vector<FunctionParameter>& params, BackrefList& nameBackrefList, string& out)
 {
-	m_logger->Indent();
+	//m_logger->Indent();
 	DemangleVariableList(params, nameBackrefList);
-	m_logger->Dedent();
+	//m_logger->Dedent();
 	m_logger->LogDebug("VariableList done\n");
 	out += "<";
 	for (size_t i = 0; i < params.size(); i++)
@@ -1564,10 +1564,10 @@ TypeBuilder Demangle::DemangleFunction(BNNameType classFunctionType, bool pointe
 
 		QualifiedName name;
 		m_logger->LogDebug("Demangle function return type %s", reader.GetRaw());
-		m_logger->Indent();
+		//m_logger->Indent();
 		returnType = DemangleVarType(nameBackrefList, true, name);
 		m_logger->LogDebug("Return type: %s", returnType.GetString().c_str());
-		m_logger->Dedent();
+		//m_logger->Dedent();
 		if (hasModifiers)
 		{
 			returnType.SetConst(return_const);
@@ -1641,9 +1641,9 @@ TypeBuilder Demangle::DemangleData()
 	m_logger->LogDebug("%s: '%s'\n", __FUNCTION__, reader.GetRaw());
 	bool _const = false, _volatile = false, isMember = false;
 	QualifiedName name;
-	m_logger->Indent();
+	//m_logger->Indent();
 	TypeBuilder newType = DemangleVarType(m_backrefList, false, name);
-	m_logger->Dedent();
+	//m_logger->Dedent();
 	auto suffix = DemanglePointerSuffix();
 	DemangleModifiers(_const, _volatile, isMember);
 	newType.SetConst(_const);
@@ -1701,7 +1701,7 @@ TypeBuilder Demangle::DemangleVTable()
 Demangle::DemangleContext Demangle::DemangleSymbol()
 {
 	m_logger->LogDebug("%s: '%s'\n", __FUNCTION__, reader.GetRaw());
-	m_logger->Indent();
+	//m_logger->Indent();
 	BNNameType classFunctionType = NoNameType;
 	QualifiedName varName;
 
diff --git a/log.cpp b/log.cpp
index e6abe48b..9edb4e4f 100644
--- a/log.cpp
+++ b/log.cpp
@@ -486,7 +486,7 @@ void Logger::Log(BNLogLevel level, const char* fmt, ...)
 {
 	va_list args;
 	va_start(args, fmt);
-	PerformLog(GetSessionId(), level, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+	PerformLog(GetSessionId(), level, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -496,7 +496,7 @@ void Logger::LogTrace(const char* fmt, ...)
 #ifdef BN_ENABLE_LOG_TRACE
 	va_list args;
 	va_start(args, fmt);
-	PerformLog(GetSessionId(), DebugLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+	PerformLog(GetSessionId(), DebugLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 #endif
 }
@@ -506,7 +506,7 @@ void Logger::LogDebug(const char* fmt, ...)
 {
 	va_list args;
 	va_start(args, fmt);
-	PerformLog(GetSessionId(), DebugLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+	PerformLog(GetSessionId(), DebugLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -515,7 +515,7 @@ void Logger::LogInfo(const char* fmt, ...)
 {
 	va_list args;
 	va_start(args, fmt);
-	PerformLog(GetSessionId(), InfoLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+	PerformLog(GetSessionId(), InfoLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -524,7 +524,7 @@ void Logger::LogWarn(const char* fmt, ...)
 {
 	va_list args;
 	va_start(args, fmt);
-	PerformLog(GetSessionId(), WarningLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+	PerformLog(GetSessionId(), WarningLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -533,7 +533,7 @@ void Logger::LogError(const char* fmt, ...)
 {
 	va_list args;
 	va_start(args, fmt);
-	PerformLog(GetSessionId(), ErrorLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+	PerformLog(GetSessionId(), ErrorLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -542,7 +542,7 @@ void Logger::LogAlert(const char* fmt, ...)
 {
 	va_list args;
 	va_start(args, fmt);
-	PerformLog(GetSessionId(), AlertLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+	PerformLog(GetSessionId(), AlertLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -552,7 +552,7 @@ void Logger::LogForException(BNLogLevel level, const std::exception& e, const ch
 	va_list args;
 	va_start(args, fmt);
 	PerformLogForException(
-		GetSessionId(), level, GetName(), GetThreadId(), e, fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), level, GetName(), GetThreadId(), e, fmt, args);
 	va_end(args);
 }
 
@@ -563,7 +563,7 @@ void Logger::LogTraceForException(const std::exception& e, const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogForException(
-		GetSessionId(), DebugLog, GetName(), GetThreadId(), e, fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), DebugLog, GetName(), GetThreadId(), e, fmt, args);
 	va_end(args);
 #endif
 }
@@ -574,7 +574,7 @@ void Logger::LogDebugForException(const std::exception& e, const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogForException(
-		GetSessionId(), DebugLog, GetName(), GetThreadId(), e, fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), DebugLog, GetName(), GetThreadId(), e, fmt, args);
 	va_end(args);
 }
 
@@ -584,7 +584,7 @@ void Logger::LogInfoForException(const std::exception& e, const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogForException(
-		GetSessionId(), InfoLog, GetName(), GetThreadId(), e, fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), InfoLog, GetName(), GetThreadId(), e, fmt, args);
 	va_end(args);
 }
 
@@ -594,7 +594,7 @@ void Logger::LogWarnForException(const std::exception& e, const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogForException(
-		GetSessionId(), WarningLog, GetName(), GetThreadId(), e, fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), WarningLog, GetName(), GetThreadId(), e, fmt, args);
 	va_end(args);
 }
 
@@ -604,7 +604,7 @@ void Logger::LogErrorForException(const std::exception& e, const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogForException(
-		GetSessionId(), ErrorLog, GetName(), GetThreadId(), e, fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), ErrorLog, GetName(), GetThreadId(), e, fmt, args);
 	va_end(args);
 }
 
@@ -614,7 +614,7 @@ void Logger::LogAlertForException(const std::exception& e, const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogForException(
-		GetSessionId(), AlertLog, GetName(), GetThreadId(), e, fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), AlertLog, GetName(), GetThreadId(), e, fmt, args);
 	va_end(args);
 }
 
@@ -624,7 +624,7 @@ void Logger::LogWithStackTrace(BNLogLevel level, const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogWithStackTrace(
-		GetSessionId(), level, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), level, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -635,7 +635,7 @@ void Logger::LogTraceWithStackTrace(const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogWithStackTrace(
-		GetSessionId(), DebugLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), DebugLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 #endif
 }
@@ -646,7 +646,7 @@ void Logger::LogDebugWithStackTrace(const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogWithStackTrace(
-		GetSessionId(), DebugLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), DebugLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -656,7 +656,7 @@ void Logger::LogInfoWithStackTrace(const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogWithStackTrace(
-		GetSessionId(), InfoLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), InfoLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -666,7 +666,7 @@ void Logger::LogWarnWithStackTrace(const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogWithStackTrace(
-		GetSessionId(), WarningLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), WarningLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -676,7 +676,7 @@ void Logger::LogErrorWithStackTrace(const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogWithStackTrace(
-		GetSessionId(), ErrorLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), ErrorLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -686,7 +686,7 @@ void Logger::LogAlertWithStackTrace(const char* fmt, ...)
 	va_list args;
 	va_start(args, fmt);
 	PerformLogWithStackTrace(
-		GetSessionId(), AlertLog, GetName(), GetThreadId(), fmt::format("{}{}", GetIndent(), fmt).c_str(), args);
+		GetSessionId(), AlertLog, GetName(), GetThreadId(), fmt, args);
 	va_end(args);
 }
 
@@ -749,37 +749,6 @@ size_t Logger::GetSessionId()
 }
 
 
-void Logger::Indent()
-{
-	BNLoggerIndent(m_object);
-}
-
-
-void Logger::Dedent()
-{
-	BNLoggerDedent(m_object);
-}
-
-
-void Logger::ResetIndent()
-{
-	BNLoggerResetIndent(m_object);
-}
-
-
-string Logger::GetIndent() const
-{
-	char* indent = BNGetLoggerIndent(m_object);
-	if (!indent)
-	{
-		return "";
-	}
-	string result = indent;
-	BNFreeString(indent);
-	return result;
-}
-
-
 Ref<Logger> LogRegistry::CreateLogger(const std::string& loggerName, size_t sessionId)
 {
 	return new Logger(BNLogCreateLogger(loggerName.c_str(), sessionId));