[PATCH 0/1] MR1128: msvcrt: Display message box in abort() in debug CRT only.

[Paul Gofman (＠gofman)]

As far as my testing goes, the message box is never shown on Windows when abort() is called with retail runtime is used, regardless of error mode and abort behaviour. That is not the case with _assert which still can show a dialog even in release mode.

It happens rather often that an app meets the error condition during the process exit and that goes silent on Windows while we display the abort dialog on app's exit.

The test program below shows that (and the same behaviour is, e. g., with ucrtbase vs ucrtbased).

```
#include <windows.h>
#include <stdio.h>
#include <stdlib.h>

void CDECL _set_app_type(int app_type);

#define D(name) static typeof(name) *p_##name
#define L(name) p_##name = (void *)GetProcAddress(hdll, #name); if (!p_##name) printf("NULL %s.\n", #name)


D(abort);
D(_set_app_type);
D(_set_abort_behavior);
D(_set_error_mode);

int main(int argc, char *argv[])
{
    HMODULE hdll = LoadLibraryA("msvcrt.dll");
    printf("hdll %p.\n", hdll);

    L(_set_app_type);
    if (!p__set_app_type)
        if (!(p__set_app_type = (void *)GetProcAddress(hdll, "__set_app_type")))
            printf("NULL p__set_app_type.\n");
    L(abort);
    L(_set_abort_behavior);
    L(_set_error_mode);

    p__set_app_type(2);
    if (p__set_abort_behavior)
        p__set_abort_behavior(_WRITE_ABORT_MSG, _WRITE_ABORT_MSG);
    p__set_error_mode(_OUT_TO_MSGBOX);
    p_abort();
}
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1128

[PATCH 1/1] msvcrt: Display message box in abort() in debug CRT only.

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

---
 dlls/msvcrt/exit.c       | 2 ++
 dlls/msvcrtd/Makefile.in | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/dlls/msvcrt/exit.c b/dlls/msvcrt/exit.c
index 323d205b1bd..643f1c109c9 100644
--- a/dlls/msvcrt/exit.c
+++ b/dlls/msvcrt/exit.c
@@ -255,7 +255,9 @@ void CDECL abort(void)
     if ((MSVCRT_error_mode == _OUT_TO_MSGBOX) ||
        ((MSVCRT_error_mode == _OUT_TO_DEFAULT) && (MSVCRT_app_type == 2)))
     {
+#ifdef _CRTDEBUG
       DoMessageBox("Runtime error!", "abnormal program termination");
+#endif
     }
     else
       _cputs("\nabnormal program termination\n");
diff --git a/dlls/msvcrtd/Makefile.in b/dlls/msvcrtd/Makefile.in
index bd4f78729d0..ca169351a0a 100644
--- a/dlls/msvcrtd/Makefile.in
+++ b/dlls/msvcrtd/Makefile.in
@@ -1,4 +1,4 @@
-EXTRADEFS = -D_CRTIMP=
+EXTRADEFS = -D_CRTIMP= -D_CRTDEBUG=
 MODULE    = msvcrtd.dll
 IMPORTLIB = msvcrtd
 IMPORTS   = ntdll
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/1128

[PATCH 1/1] msvcrt: Display message box in abort() in debug CRT only.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=125218

Your paranoid android.


=== debian11 (build log) ===

Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24736.
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24736.
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24736.

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcrt/exit.c:
> if ((MSVCRT_error_mode == _OUT_TO_MSGBOX) ||
>         ((MSVCRT_error_mode == _OUT_TO_DEFAULT) && (MSVCRT_app_type == 2)))
>      {
> +#ifdef _CRTDEBUG
>        DoMessageBox("Runtime error!", "abnormal program termination");
> +#endif

The message box should be displayed in older versions of msvcr dlls (e.g. msvcr90). ucrtbase (I didn't test other versions) is also not printing any message to console.

Did you consider defining _DEBUG instead of _CRTDEBUG?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1128#note_11675

[Paul Gofman]

Thanks, I will check more with old redists.

Regarding _DEBUG, this is the standard one used in debug related macros. I thought using the same in internal CRT implementation may potentially introduce ambiguity (although should work). Do you think it is better to use that one?
> On 21 Oct 2022, at 04:48, Piotr Caban (@piotr) <wine(a)gitlab.winehq.org> wrote:
>
> Piotr Caban (@piotr) commented about dlls/msvcrt/exit.c:
> > if ((MSVCRT_error_mode == _OUT_TO_MSGBOX) ||
> >        ((MSVCRT_error_mode == _OUT_TO_DEFAULT) && (MSVCRT_app_type == 2)))
> >     {
> > +#ifdef _CRTDEBUG
> >       DoMessageBox("Runtime error!", "abnormal program termination");
> > +#endif
>
> The message box should be displayed in older versions of msvcr dlls (e.g. msvcr90). ucrtbase (I didn't test other versions) is also not printing any message to console.
>
> Did you consider defining _DEBUG instead of _CRTDEBUG?
>
> -- 
> https://gitlab.winehq.org/wine/wine/-/merge_requests/1128#note_11675

[Piotr Caban (＠piotr)]

On Fri Oct 21 10:12:33 2022 +0000, **** wrote:
> Paul Gofman replied on the mailing list:
> ```
> Thanks, I will check more with old redists.
> Regarding _DEBUG, this is the standard one used in debug related macros.
> I thought using the same in internal CRT implementation may potentially
> introduce ambiguity (although should work). Do you think it is better to
> use that one?
> ```
> I would expect, that if there are any differences in headers, it will be guarded by _DEBUG check. That's why it might be useful to use it instead of inventing something new. I didn't check if _DEBUG is used in this way in C-runtimes headers.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1128#note_11689