1
0
forked from 0ad/0ad

Reduces number of allocations during error message formatting.

Differential Revision: https://code.wildfiregames.com/D3871
This was SVN commit r25300.
This commit is contained in:
Vladislav Belov 2021-04-22 06:53:03 +00:00
parent 19e055c6e1
commit f4905066ab
4 changed files with 46 additions and 90 deletions

View File

@ -1,4 +1,4 @@
/* Copyright (C) 2010 Wildfire Games.
/* Copyright (C) 2021 Wildfire Games.
*
* Permission is hereby granted, free of charge, to any person obtaining
* a copy of this software and associated documentation files (the
@ -20,27 +20,33 @@
* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
/*
* platform-independent debug support code.
*/
#include "precompiled.h"
#include "lib/debug.h"
#include "lib/alignment.h"
#include "lib/app_hooks.h"
#include "lib/fnv_hash.h"
#include "lib/sysdep/cpu.h" // cpu_CAS
#include "lib/sysdep/sysdep.h"
#include "lib/sysdep/vm.h"
#if OS_WIN
# include "lib/sysdep/os/win/wdbg_heap.h"
#endif
#include <cstdarg>
#include <cstring>
#include <cstdio>
#include "lib/alignment.h"
#include "lib/app_hooks.h"
#include "lib/fnv_hash.h"
#include "lib/sysdep/vm.h"
#include "lib/sysdep/cpu.h" // cpu_CAS
#include "lib/sysdep/sysdep.h"
namespace
{
#if OS_WIN
# include "lib/sysdep/os/win/wdbg_heap.h"
#endif
// (NB: this may appear obscene, but deep stack traces have been
// observed to take up > 256 KiB)
constexpr std::size_t MESSAGE_SIZE = 512 * KiB / sizeof(wchar_t);
wchar_t g_MessageBuffer[MESSAGE_SIZE];
} // anonymous namespace
static const StatusDefinition debugStatusDefinitions[] = {
{ ERR::SYM_NO_STACK_FRAMES_FOUND, L"No stack frames found" },
@ -207,15 +213,6 @@ Status debug_WriteCrashlog(const wchar_t* text)
// error message
//-----------------------------------------------------------------------------
// (NB: this may appear obscene, but deep stack traces have been
// observed to take up > 256 KiB)
static const size_t messageSize = 512*KiB;
void debug_FreeErrorMessage(ErrorMessageMem* emm)
{
vm::Free(emm->pa_mem, messageSize);
}
// a stream with printf-style varargs and the possibility of
// writing directly to the output buffer.
@ -267,8 +264,7 @@ private:
const wchar_t* debug_BuildErrorMessage(
const wchar_t* description,
const wchar_t* filename, int line, const char* func,
void* context, const wchar_t* lastFuncToSkip,
ErrorMessageMem* emm)
void* context, const wchar_t* lastFuncToSkip)
{
// retrieve errno (might be relevant) before doing anything else
// that might overwrite it.
@ -279,12 +275,7 @@ const wchar_t* debug_BuildErrorMessage(
StatusDescription(errno_equiv, description_buf, ARRAY_SIZE(description_buf));
sys_StatusDescription(0, os_error, ARRAY_SIZE(os_error));
// rationale: see ErrorMessageMem
emm->pa_mem = vm::Allocate(messageSize);
wchar_t* const buf = (wchar_t*)emm->pa_mem;
if(!buf)
return L"(insufficient memory to generate error message)";
PrintfWriter writer(buf, messageSize / sizeof(wchar_t));
PrintfWriter writer(g_MessageBuffer, MESSAGE_SIZE);
// header
if(!writer(
@ -334,7 +325,7 @@ fail:
))
goto fail;
return buf;
return g_MessageBuffer;
}
@ -465,21 +456,18 @@ ErrorReaction debug_DisplayError(const wchar_t* description,
const wchar_t* filename = path_name_only(pathname);
// display in output window; double-click will navigate to error location.
debug_printf("%s(%d): %s\n", utf8_from_wstring(filename).c_str(), line, utf8_from_wstring(description).c_str());
ErrorMessageMem emm;
const wchar_t* text = debug_BuildErrorMessage(description, filename, line, func, context, lastFuncToSkip, &emm);
const wchar_t* text = debug_BuildErrorMessage(description, filename, line, func, context, lastFuncToSkip);
(void)debug_WriteCrashlog(text);
ErrorReactionInternal er = CallDisplayError(text, flags);
// TODO: use utf8 conversion without internal allocations.
debug_printf("%s(%d): %s\n", utf8_from_wstring(filename).c_str(), line, utf8_from_wstring(description).c_str());
// note: debug_break-ing here to make sure the app doesn't continue
// running is no longer necessary. debug_DisplayError now determines our
// window handle and is modal.
// must happen before PerformErrorReaction because that may exit.
debug_FreeErrorMessage(&emm);
return PerformErrorReaction(er, flags, suppress);
}
@ -535,7 +523,6 @@ static bool ShouldSkipError(Status err)
return false;
}
ErrorReaction debug_OnError(Status err, atomic_bool* suppress, const wchar_t* file, int line, const char* func)
{
CACHE_ALIGNED(u8) context[DEBUG_CONTEXT_SIZE];
@ -551,14 +538,13 @@ ErrorReaction debug_OnError(Status err, atomic_bool* suppress, const wchar_t* fi
return debug_DisplayError(buf, DE_MANUAL_BREAK, context, lastFuncToSkip, file,line,func, suppress);
}
ErrorReaction debug_OnAssertionFailure(const wchar_t* expr, atomic_bool* suppress, const wchar_t* file, int line, const char* func)
{
CACHE_ALIGNED(u8) context[DEBUG_CONTEXT_SIZE];
(void)debug_CaptureContext(context);
const std::wstring lastFuncToSkip = L"debug_OnAssertionFailure";
const wchar_t* lastFuncToSkip = L"debug_OnAssertionFailure";
wchar_t buf[400];
swprintf_s(buf, ARRAY_SIZE(buf), L"Assertion failed: \"%ls\"", expr);
return debug_DisplayError(buf, DE_MANUAL_BREAK, context, lastFuncToSkip.c_str(), file,line,func, suppress);
return debug_DisplayError(buf, DE_MANUAL_BREAK, context, lastFuncToSkip, file,line,func, suppress);
}

View File

@ -1,4 +1,4 @@
/* Copyright (C) 2020 Wildfire Games.
/* Copyright (C) 2021 Wildfire Games.
*
* Permission is hereby granted, free of charge, to any person obtaining
* a copy of this software and associated documentation files (the
@ -36,12 +36,12 @@
// - the output routines make for platform-independent logging and
// crashlogs with "last-known activity" reporting.
#include "lib/lib_api.h"
#include "lib/types.h" // intptr_t
#include "lib/status.h"
#include "lib/alignment.h"
#include "lib/code_annotation.h"
#include "lib/code_generation.h"
#include "lib/lib_api.h"
#include "lib/status.h"
#include "lib/types.h"
/**
* trigger a breakpoint when reached/"called".
@ -537,31 +537,6 @@ LIB_API bool debug_IsStackPointer(void* p);
**/
LIB_API void debug_SetThreadName(const char* name);
/**
* holds memory for an error message.
**/
struct ErrorMessageMem
{
// rationale:
// - error messages with stack traces require a good deal of memory
// (hundreds of KB). static buffers of that size are undesirable.
// - the heap may be corrupted, so don't use malloc.
// instead, "lib/sysdep/vm.h" functions should be safe.
// - alloca is a bit iffy (the stack may be maxed out), non-portable and
// complicates the code because it can't be allocated by a subroutine.
// - this method is probably slow, but error messages aren't built often.
// if necessary, first try malloc and use mmap if that fails.
void* pa_mem;
};
/**
* free memory from the error message.
*
* @param emm ErrorMessageMem*
**/
LIB_API void debug_FreeErrorMessage(ErrorMessageMem* emm);
/**
* build a string describing the given error.
*
@ -572,11 +547,7 @@ LIB_API void debug_FreeErrorMessage(ErrorMessageMem* emm);
* @param fn_only filename (no path) of source file that triggered the error.
* @param line, func: exact position of the error.
* @param context, lastFuncToSkip: see debug_DumpStack.
* @param emm memory for the error message. caller should allocate
* stack memory and set alloc_buf*; if not, there will be no
* fallback in case heap alloc fails. should be freed via
* debug_FreeErrorMessage when no longer needed.
**/
LIB_API const wchar_t* debug_BuildErrorMessage(const wchar_t* description, const wchar_t* fn_only, int line, const char* func, void* context, const wchar_t* lastFuncToSkip, ErrorMessageMem* emm);
LIB_API const wchar_t* debug_BuildErrorMessage(const wchar_t* description, const wchar_t* fn_only, int line, const char* func, void* context, const wchar_t* lastFuncToSkip);
#endif // #ifndef INCLUDED_DEBUG

View File

@ -111,18 +111,11 @@ class TestWdbgSym : public CxxTest::TestSuite
// note: we don't want any kind of dialog to be raised, because
// this test now always runs. therefore, just make sure a decent
// amount of text (not just "(failed)" error messages) was produced.
ErrorMessageMem emm = {0};
CACHE_ALIGNED(u8) context[DEBUG_CONTEXT_SIZE];
TS_ASSERT_EQUALS(debug_CaptureContext(context), INFO::OK);
const wchar_t* text = debug_BuildErrorMessage(L"dummy", 0,0,0, context, L"m_test_array", &emm);
const wchar_t* text = debug_BuildErrorMessage(L"dummy", 0, 0, 0, context, L"m_test_array");
TS_ASSERT(wcslen(text) > 500);
#if 0
{
std::wofstream s(L"d:\\out.txt");
s << text;
}
#endif
debug_FreeErrorMessage(&emm);
debug_printf("(done dumping stack frames)\n");
}

View File

@ -361,22 +361,28 @@ Status sys_StatusDescription(int user_err, wchar_t* buf, size_t max_chars)
if(user_err < 0)
return ERR::FAIL; // NOWARN
const DWORD err = user_err? (DWORD)user_err : GetLastError();
const DWORD errorCode = user_err? (DWORD)user_err : GetLastError();
// no one likes to see "The operation completed successfully" in
// error messages, so return more descriptive text instead.
if(err == 0)
if(errorCode == 0)
{
wcscpy_s(buf, max_chars, L"0 (no error code was set)");
return INFO::OK;
}
wchar_t message[400];
if(errorCode == ERROR_NOT_ENOUGH_MEMORY)
{
// We handle out of memory separately to prevent possible subsequent/nested allocations.
swprintf_s(message, ARRAY_SIZE(message), L"Not enough memory resources are available to process this command.");
}
else
{
const LPCVOID source = 0; // ignored (we're not using FROM_HMODULE etc.)
const DWORD lang_id = 0; // look for neutral, then current locale
va_list* args = 0; // we don't care about "inserts"
const DWORD charsWritten = FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM, source, err, lang_id, message, (DWORD)ARRAY_SIZE(message), args);
const DWORD charsWritten = FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM, source, errorCode, lang_id, message, (DWORD)ARRAY_SIZE(message), args);
if(!charsWritten)
WARN_RETURN(ERR::FAIL);
ENSURE(charsWritten < max_chars);
@ -386,7 +392,7 @@ Status sys_StatusDescription(int user_err, wchar_t* buf, size_t max_chars)
message[charsWritten-2] = '\0';
}
const int charsWritten = swprintf_s(buf, max_chars, L"%d (%ls)", err, message);
const int charsWritten = swprintf_s(buf, max_chars, L"%d (%ls)", errorCode, message);
ENSURE(charsWritten != -1);
return INFO::OK;
}