Re: [Ping] Port of VTV for Cygwin and MinGW

[Kai Tietz <ktietz70 at googlemail dot com>]

2014-11-12 18:45 GMT+01:00 Patrick Wollgast <patrick.wollgast@rub.de>:
>>
>> I don't think you have addressed all of the comments I made in the
>> comment, do you?
>>
>> Regards,
>> Kai
>>
>
> I added the three checks, if TARGET_PECOFF is defined, and fixed the
> whitespace issues.
>
> For the questions regarding C-runtime/Win32 functions I haven't changed
> anything in the patch but explained why I used those functions.
>
>>> Why you use instead of C-runtime exit/abort-functions the
>>> platform-functions to terminate the process.  This looks to me like
>>> useless change.  For cygwin this might be even wrong in some aspects.
>>> What is the reasoning for this change?
>>>
>>
>> I haven't encountered crashes in obstack.c itself, but there were
>> problems in vtv_rts.cc with abort() on MinGW 32bit. The following stack
>> traces were taken at MinGW 32bit. Most of the time the process had to be
>> stopped in the process manager because a wrong process handle was passed
>> to NtTerminateProcess. This was tested and occurred on Windows 7 64bit
>> and Windows 8.1 64bit. To be sure to avoid this issue the calls have
>> also been exchanged in obstack.c.
>>
>> With abort(), correct process handle, postmortem debugger triggert.
>>
>> ffffffff 00000003 0028fe98 ntdll!NtTerminateProcess+0xc
>> 00000003 77e8f3b0 ffffffff ntdll!RtlExitUserProcess+0x6d
>> 00000003 0028f934 74f85472 KERNEL32!ExitProcessImplementation+0x12
>> 00000003 11e9bfd9 00409000 msvcrt!exit+0x32
>> 00000003 00000001 00000000 msvcrt!flushall+0x2e9
>> 00000003 00010001 00000065 msvcrt!exit+0x11
>> 6efcf294 00000080 0028ffcc msvcrt!abort+0xf3
>> 00560f70 0000000d 00000001 libvtv_0!Z14__fortify_failPKc+0x18
>> 7ffde000 0028ffdc 77568f8b test_std+0x13de
>> 7ffde000 138a1dee 00000000 KERNEL32!BaseThreadInitThunk+0xe
>> ffffffff 7755dad3 00000000 ntdll!__RtlUserThreadStart+0x20
>> 004014e0 7ffde000 00000000 ntdll!_RtlUserThreadStart+0x1b
>>
>> With abort(), wrong process handle, NtTerminateProcess returns instead
>> of ending the own process. This case happens most of the time.
>>
>> 00000000 00000003 0028fe98 ntdll!NtTerminateProcess+0x5
>> 00000003 77e8f3b0 ffffffff ntdll!RtlExitUserProcess+0x35
>> 00000003 0028f934 74f85472 KERNEL32!ExitProcessImplementation+0x12
>> 00000003 9f9f5ea3 00409000 msvcrt!exit+0x32
>> 00000003 00000001 00000000 msvcrt!flushall+0x2e9
>> 00000003 00010001 00000040 msvcrt!exit+0x11
>> 6efcf294 00000080 0028ffcc msvcrt!abort+0xf3
>> 00701060 0000001e 00000001 libvtv_0!Z14__fortify_failPKc+0x18
>> 7ffde000 0028ffdc 77568f8b image00400000+0x13de
>> 7ffde000 9defd39c 00000000 KERNEL32!BaseThreadInitThunk+0xe
>> ffffffff 7755dac5 00000000 ntdll!__RtlUserThreadStart+0x20
>> 004014e0 7ffde000 00000000 ntdll!_RtlUserThreadStart+0x1b
>>
>> TerminateProcess. Everything's fine on MinGW x86, x86-64 (both gcc 5.0),
>> Cygwin x86-64 (gcc 4.9.0).
>>
>> ffffffff 0000022f 0028feb8 ntdll!NtTerminateProcess+0x5
>> ffffffff 0000022f 00409000 KERNELBASE!TerminateProcess+0x27
>> 00731060 0000001e 00000001 libvtv_0!Z14__fortify_failPKc+0x2a
>> 7ffde000 0028ffdc 77568f8b image00400000+0x13de
>> 7ffde000 e2645ec6 00000000 KERNEL32!BaseThreadInitThunk+0xe
>> ffffffff 7755dae7 00000000 ntdll!__RtlUserThreadStart+0x20
>> 004014e0 7ffde000 00000000 ntdll!_RtlUserThreadStart+0x1b

TerminateProcess is actually bad, as it doesn't call any of the atexit
handlers.  You simply nuke the process off.  For cygwin this behavior
is inacceptable.  Why a classical abort, or a classical exit call
cause for you that issues?  It seems to me more related to some other
thing you try to paper over by this.


> Regarding the question, why I reimplemented mprotect, I also haven't
> changed anything in the patch but answered the question.

And this doesn't make it better.  It is present in the static part of
libgcc.  Have you tried to declare it with extern "C" (for C++ case)
and simply use it?
Cygwin provides its own version too.  So there seems to me no real
need to re-implement it.

>>> Another note I have about re-implementation of mprotect in ---
>>> libvtv/vtv_malloc.cc.  Why you need that?  it is already part of
>>> libgcc for mingw.  And for cygwin this function is part of cygwin's
>>> library itself.  So why re-implementing it here?
>>>
>>
>> It is already part of libgcc for MinGW, but it can neither be found in
>> the exports of the dll, nor can the function prototype be found in any
>> header files. Therefore I get unknown reference errors if I don't
>> re-implement it. I checked the exports of several compilations for this.
>>
>> * self compiled MinGW
>> * MinGW installed from the Arch Linux repositories
>> *
>>
> http://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Automated%20Builds/mingw-w64-bin_x86_64-linux_20131228.tar.bz2/download
>>
>> If I'm missing something here let me know.
>
> I think this was everything you addressed.
>
> Regards,
> Patrick

Regards,
Kai