ci: make deploying builds globally blocking, don't cancel in-progress

Catch CLR exceptions

.github/workflows/main.yml

@@ -1,9 +1,10 @@
 name: Build Dalamud
 on: [push, pull_request, workflow_dispatch]
 
+# Globally blocking because of git pushes in deploy step
 concurrency:
-  group: build_dalamud_${{ github.ref_name }}
-  cancel-in-progress: true
+  group: build_dalamud_${{ github.repository_owner }}
+  cancel-in-progress: false
 
 jobs:
   build:

Dalamud.Boot/crashhandler_shared.h

@@ -6,6 +6,8 @@
 #define WIN32_LEAN_AND_MEAN
 #include <Windows.h>
 
+#define CUSTOM_EXCEPTION_EXTERNAL_EVENT 0x12345679
+
 struct exception_info
 {
     LPEXCEPTION_POINTERS pExceptionPointers;

Dalamud.Boot/dllmain.cpp

@@ -331,6 +331,51 @@ HRESULT WINAPI InitializeImpl(LPVOID lpParam, HANDLE hMainThreadContinue) {
         logging::I("VEH was disabled manually");
     }
 
+    // ============================== CLR Reporting =================================== //
+
+    // This is pretty horrible - CLR just doesn't provide a way for us to handle these events, and the API for it
+    // was pushed back to .NET 11, so we have to hook ReportEventW and catch them ourselves for now.
+    // Ideally all of this will go away once they get to it.
+    static std::shared_ptr<hooks::global_import_hook<decltype(ReportEventW)>> s_report_event_hook;
+    s_report_event_hook = std::make_shared<hooks::global_import_hook<decltype(ReportEventW)>>(
+        "advapi32.dll!ReportEventW (global import, hook_clr_report_event)", L"advapi32.dll", "ReportEventW");
+    s_report_event_hook->set_detour([hook = s_report_event_hook.get()](
+        HANDLE hEventLog,
+        WORD wType,
+        WORD wCategory,
+        DWORD dwEventID,
+        PSID lpUserSid,
+        WORD wNumStrings,
+        DWORD dwDataSize,
+        LPCWSTR* lpStrings,
+        LPVOID lpRawData)-> BOOL {
+
+        // Check for CLR Error Event IDs
+        // https://github.com/dotnet/runtime/blob/v10.0.0/src/coreclr/vm/eventreporter.cpp#L370
+        if (dwEventID != 1026 && // ERT_UnhandledException: The process was terminated due to an unhandled exception
+            dwEventID != 1025 && // ERT_ManagedFailFast: The application requested process termination through System.Environment.FailFast
+            dwEventID != 1023 && // ERT_UnmanagedFailFast: The process was terminated due to an internal error in the .NET Runtime
+            dwEventID != 1027 && // ERT_StackOverflow: The process was terminated due to a stack overflow
+            dwEventID != 1028)   // ERT_CodeContractFailed: The application encountered a bug.  A managed code contract (precondition, postcondition, object invariant, or assert) failed
+        {
+            return hook->call_original(hEventLog, wType, wCategory, dwEventID, lpUserSid, wNumStrings, dwDataSize, lpStrings, lpRawData);
+        }
+
+        if (wNumStrings == 0 || lpStrings == nullptr) {
+            logging::W("ReportEventW called with no strings.");
+            return hook->call_original(hEventLog, wType, wCategory, dwEventID, lpUserSid, wNumStrings, dwDataSize, lpStrings, lpRawData);
+        }
+
+        // In most cases, DalamudCrashHandler will kill us now, so call original here to make sure we still write to the event log.
+        const BOOL original_ret = hook->call_original(hEventLog, wType, wCategory, dwEventID, lpUserSid, wNumStrings, dwDataSize, lpStrings, lpRawData);
+
+        const std::wstring error_details(lpStrings[0]);
+        veh::raise_external_event(error_details);
+
+        return original_ret;
+    });
+    logging::I("ReportEventW hook installed.");
+
     // ============================== Dalamud ==================================== //
 
     if (static_cast<int>(g_startInfo.BootWaitMessageBox) & static_cast<int>(DalamudStartInfo::WaitMessageboxFlags::BeforeDalamudEntrypoint))

Dalamud.Boot/veh.cpp

@@ -31,6 +31,8 @@ HANDLE g_crashhandler_process = nullptr;
 HANDLE g_crashhandler_event = nullptr;
 HANDLE g_crashhandler_pipe_write = nullptr;
 
+wchar_t g_external_event_info[16384] = L"";
+
 std::recursive_mutex g_exception_handler_mutex;
 
 std::chrono::time_point<std::chrono::system_clock> g_time_start;
@@ -190,7 +192,11 @@ LONG exception_handler(EXCEPTION_POINTERS* ex)
     DuplicateHandle(GetCurrentProcess(), g_crashhandler_event, g_crashhandler_process, &exinfo.hEventHandle, 0, TRUE, DUPLICATE_SAME_ACCESS);
 
     std::wstring stackTrace;
-    if (!g_clr)
+    if (ex->ExceptionRecord->ExceptionCode == CUSTOM_EXCEPTION_EXTERNAL_EVENT)
+    {
+        stackTrace = std::wstring(g_external_event_info);
+    }
+    else if (!g_clr)
     {
         stackTrace = L"(no CLR stack trace available)";
     }
@@ -251,6 +257,12 @@ LONG WINAPI structured_exception_handler(EXCEPTION_POINTERS* ex)
 
 LONG WINAPI vectored_exception_handler(EXCEPTION_POINTERS* ex)
 {
+    // special case for CLR exceptions, always trigger crash handler
+    if (ex->ExceptionRecord->ExceptionCode == CUSTOM_EXCEPTION_EXTERNAL_EVENT)
+    {
+        return exception_handler(ex);
+    }
+
     if (ex->ExceptionRecord->ExceptionCode == 0x12345678)
     {
         // pass
@@ -434,3 +446,10 @@ bool veh::remove_handler()
     }
     return false;
 }
+
+void veh::raise_external_event(const std::wstring& info)
+{
+    const auto info_size = std::min(info.size(), std::size(g_external_event_info) - 1);
+    wcsncpy_s(g_external_event_info, info.c_str(), info_size);
+    RaiseException(CUSTOM_EXCEPTION_EXTERNAL_EVENT, 0, 0, nullptr);
+}

Dalamud.Boot/veh.h

@@ -4,4 +4,5 @@ namespace veh
 {
     bool add_handler(bool doFullDump, const std::string& workingDirectory);
     bool remove_handler();
+    void raise_external_event(const std::wstring& info);
 }

Dalamud/Game/Addon/Events/AddonEventManager.cs

@@ -9,7 +9,6 @@ using Dalamud.Logging.Internal;
 using Dalamud.Plugin.Internal.Types;
 using Dalamud.Plugin.Services;
 
-using FFXIVClientStructs.FFXIV.Client.UI;
 using FFXIVClientStructs.FFXIV.Component.GUI;
 
 namespace Dalamud.Game.Addon.Events;
@@ -32,25 +31,21 @@ internal unsafe class AddonEventManager : IInternalDisposableService
 
     private readonly AddonLifecycleEventListener finalizeEventListener;
 
-    private readonly AddonEventManagerAddressResolver address;
-    private readonly Hook<UpdateCursorDelegate> onUpdateCursor;
+    private readonly Hook<AtkUnitManager.Delegates.UpdateCursor> onUpdateCursor;
 
     private readonly ConcurrentDictionary<Guid, PluginEventController> pluginEventControllers;
 
-    private AddonCursorType? cursorOverride;
+    private AtkCursor.CursorType? cursorOverride;
 
     [ServiceManager.ServiceConstructor]
-    private AddonEventManager(TargetSigScanner sigScanner)
+    private AddonEventManager()
     {
-        this.address = new AddonEventManagerAddressResolver();
-        this.address.Setup(sigScanner);
-
         this.pluginEventControllers = new ConcurrentDictionary<Guid, PluginEventController>();
         this.pluginEventControllers.TryAdd(DalamudInternalKey, new PluginEventController());
 
         this.cursorOverride = null;
 
-        this.onUpdateCursor = Hook<UpdateCursorDelegate>.FromAddress(this.address.UpdateCursor, this.UpdateCursorDetour);
+        this.onUpdateCursor = Hook<AtkUnitManager.Delegates.UpdateCursor>.FromAddress(AtkUnitManager.Addresses.UpdateCursor.Value, this.UpdateCursorDetour);
 
         this.finalizeEventListener = new AddonLifecycleEventListener(AddonEvent.PreFinalize, string.Empty, this.OnAddonFinalize);
         this.addonLifecycle.RegisterListener(this.finalizeEventListener);
@@ -58,8 +53,6 @@ internal unsafe class AddonEventManager : IInternalDisposableService
         this.onUpdateCursor.Enable();
     }
 
-    private delegate nint UpdateCursorDelegate(RaptureAtkModule* module);
-
     /// <inheritdoc/>
     void IInternalDisposableService.DisposeService()
     {
@@ -117,7 +110,7 @@ internal unsafe class AddonEventManager : IInternalDisposableService
     /// Force the game cursor to be the specified cursor.
     /// </summary>
     /// <param name="cursor">Which cursor to use.</param>
-    internal void SetCursor(AddonCursorType cursor) => this.cursorOverride = cursor;
+    internal void SetCursor(AddonCursorType cursor) => this.cursorOverride = (AtkCursor.CursorType)cursor;
 
     /// <summary>
     /// Un-forces the game cursor.
@@ -168,7 +161,7 @@ internal unsafe class AddonEventManager : IInternalDisposableService
         }
     }
 
-    private nint UpdateCursorDetour(RaptureAtkModule* module)
+    private void UpdateCursorDetour(AtkUnitManager* thisPtr)
     {
         try
         {
@@ -176,13 +169,14 @@ internal unsafe class AddonEventManager : IInternalDisposableService
 
             if (this.cursorOverride is not null && atkStage is not null)
             {
-                var cursor = (AddonCursorType)atkStage->AtkCursor.Type;
-                if (cursor != this.cursorOverride)
+                ref var atkCursor = ref atkStage->AtkCursor;
+
+                if (atkCursor.Type != this.cursorOverride)
                 {
-                    AtkStage.Instance()->AtkCursor.SetCursorType((AtkCursor.CursorType)this.cursorOverride, 1);
+                    atkCursor.SetCursorType((AtkCursor.CursorType)this.cursorOverride, 1);
                 }
 
-                return nint.Zero;
+                return;
             }
         }
         catch (Exception e)
@@ -190,7 +184,7 @@ internal unsafe class AddonEventManager : IInternalDisposableService
             Log.Error(e, "Exception in UpdateCursorDetour.");
         }
 
-        return this.onUpdateCursor!.Original(module);
+        this.onUpdateCursor!.Original(thisPtr);
     }
 }
 

Dalamud/Game/Addon/Events/AddonEventManagerAddressResolver.cs

@@ -1,21 +0,0 @@
-namespace Dalamud.Game.Addon.Events;
-
-/// <summary>
-/// AddonEventManager memory address resolver.
-/// </summary>
-internal class AddonEventManagerAddressResolver : BaseAddressResolver
-{
-    /// <summary>
-    /// Gets the address of the AtkModule UpdateCursor method.
-    /// </summary>
-    public nint UpdateCursor { get; private set; }
-
-    /// <summary>
-    /// Scan for and setup any configured address pointers.
-    /// </summary>
-    /// <param name="scanner">The signature scanner to facilitate setup.</param>
-    protected override void Setup64Bit(ISigScanner scanner)
-    {
-        this.UpdateCursor = scanner.ScanText("48 89 74 24 ?? 48 89 7C 24 ?? 41 56 48 83 EC 20 4C 8B F1 E8 ?? ?? ?? ?? 49 8B CE"); // unnamed in CS
-    }
-}

DalamudCrashHandler/DalamudCrashHandler.cpp

@@ -292,35 +292,43 @@ std::wstring to_address_string(const DWORD64 address, const bool try_ptrderef =
 
 void print_exception_info(HANDLE hThread, const EXCEPTION_POINTERS& ex, const CONTEXT& ctx, std::wostringstream& log) {
     std::vector<EXCEPTION_RECORD> exRecs;
-    if (ex.ExceptionRecord) {
+    if (ex.ExceptionRecord)
+    {
         size_t rec_index = 0;
         size_t read;
-        exRecs.emplace_back();
+
         for (auto pRemoteExRec = ex.ExceptionRecord;
-             pRemoteExRec
-             && rec_index < 64
-             && ReadProcessMemory(g_hProcess, pRemoteExRec, &exRecs.back(), sizeof exRecs.back(), &read)
-             && read >= offsetof(EXCEPTION_RECORD, ExceptionInformation)
-             && read >= static_cast<size_t>(reinterpret_cast<const char*>(&exRecs.back().ExceptionInformation[exRecs.back().NumberParameters]) - reinterpret_cast<const char*>(&exRecs.back()));
-             rec_index++) {
+             pRemoteExRec && rec_index < 64;
+             rec_index++)
+        {
+            exRecs.emplace_back();
+
+            if (!ReadProcessMemory(g_hProcess, pRemoteExRec, &exRecs.back(), sizeof exRecs.back(), &read)
+                || read < offsetof(EXCEPTION_RECORD, ExceptionInformation)
+                || read < static_cast<size_t>(reinterpret_cast<const char*>(&exRecs.back().ExceptionInformation[exRecs.
+                    back().NumberParameters]) - reinterpret_cast<const char*>(&exRecs.back())))
+            {
+                exRecs.pop_back();
+                break;
+            }
 
             log << std::format(L"\nException Info #{}\n", rec_index);
             log << std::format(L"Address: {:X}\n", exRecs.back().ExceptionCode);
             log << std::format(L"Flags: {:X}\n", exRecs.back().ExceptionFlags);
             log << std::format(L"Address: {:X}\n", reinterpret_cast<size_t>(exRecs.back().ExceptionAddress));
-            if (!exRecs.back().NumberParameters)
-                continue;
-            log << L"Parameters: ";
-            for (DWORD i = 0; i < exRecs.back().NumberParameters; ++i) {
-                if (i != 0)
-                    log << L", ";
-                log << std::format(L"{:X}", exRecs.back().ExceptionInformation[i]);
+            if (exRecs.back().NumberParameters)
+            {
+                log << L"Parameters: ";
+                for (DWORD i = 0; i < exRecs.back().NumberParameters; ++i)
+                {
+                    if (i != 0)
+                        log << L", ";
+                    log << std::format(L"{:X}", exRecs.back().ExceptionInformation[i]);
+                }
             }
 
             pRemoteExRec = exRecs.back().ExceptionRecord;
-            exRecs.emplace_back();
         }
-        exRecs.pop_back();
     }
 
     log << L"\nCall Stack\n{";
@@ -930,9 +938,19 @@ int main() {
             } while (false);
         }
 
+        const bool is_external_event = exinfo.ExceptionRecord.ExceptionCode == CUSTOM_EXCEPTION_EXTERNAL_EVENT;
+
         std::wostringstream log;
-        log << std::format(L"Unhandled native exception occurred at {}", to_address_string(exinfo.ContextRecord.Rip, false)) << std::endl;
-        log << std::format(L"Code: {:X}", exinfo.ExceptionRecord.ExceptionCode) << std::endl;
+
+        if (!is_external_event)
+        {
+            log << std::format(L"Unhandled native exception occurred at {}", to_address_string(exinfo.ContextRecord.Rip, false)) << std::endl;
+            log << std::format(L"Code: {:X}", exinfo.ExceptionRecord.ExceptionCode) << std::endl;
+        }
+        else
+        {
+            log << L"CLR error occurred" << std::endl;
+        }
 
         if (shutup)
             log << L"======= Crash handler was globally muted(shutdown?) =======" << std::endl;
@@ -949,9 +967,19 @@ int main() {
         if (pProgressDialog)
             pProgressDialog->SetLine(3, L"Refreshing Module List", FALSE, NULL);
 
+        std::wstring window_log_str;
+
+        // Cut the log here for external events, the rest is unreadable and doesn't matter since we can't get
+        // symbols for mixed-mode stacks yet.
+        if (is_external_event)
+            window_log_str = log.str();
+
         SymRefreshModuleList(GetCurrentProcess());
         print_exception_info(exinfo.hThreadHandle, exinfo.ExceptionPointers, exinfo.ContextRecord, log);
-        const auto window_log_str = log.str();
+
+        if (!is_external_event)
+            window_log_str = log.str();
+
         print_exception_info_extended(exinfo.ExceptionPointers, exinfo.ContextRecord, log);
         std::wofstream(logPath) << log.str();
 

lib/FFXIVClientStructs

@@ -1 +1 @@
-Subproject commit e5dedba42a3fea8f050ea54ac583a5874bf51c6f
+Subproject commit 6f339d8f725fa6922449f7e5c584ca6b8fa2fb19