[PATCH v2 1/3] ntdll: Implement NtQueryInformationThread(ThreadTimes) using procfs.

Zebediah Figura z.figura12 at gmail.com
Mon Jun 29 09:55:36 CDT 2020


On 6/29/20 9:45 AM, Rémi Bernon wrote:
> On 2020-06-29 16:43, Zebediah Figura wrote:
>> On 6/29/20 1:32 AM, Rémi Bernon wrote:
>>> On 2020-06-29 03:43, Zebediah Figura wrote:
>>>> From: Sebastian Lackner <sebastian at fds-team.de>
>>>>
>>>> Based on a patch by Ray Hinchliffe <ray at pobox.co.uk>.
>>>>
>>>> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=20230
>>>> Signed-off-by: Zebediah Figura <z.figura12 at gmail.com>
>>>> ---
>>>> v2: Rebase onto current git (i.e. onto ntdll.so).
>>>>
>>>> This patch lets System Information Viewer, Ollydbg 1.x/2.x, and x64dbg
>>>> display
>>>> accurate values.
>>>>
>>>> While these values are available through host utilities such as ps(1),
>>>> it is
>>>> sometimes more useful to access them from within debuggers and
>>>> utilities running
>>>> under Wine.
>>>>
>>>> I have omitted NtQueryInformationProcess(ProcessTimes), since the
>>>> obvious
>>>> solution needs the server to pass back more data than it currently
>>>> can, and I
>>>> don't know what the preferred way to handle that is.
>>>>
>>>>    dlls/ntdll/unix/thread.c | 82
>>>> +++++++++++++++++++++++++++++++---------
>>>>    server/protocol.def      |  2 +
>>>>    server/thread.c          |  2 +
>>>>    3 files changed, 69 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c
>>>> index f3dddd2b02..54483e1f99 100644
>>>> --- a/dlls/ntdll/unix/thread.c
>>>> +++ b/dlls/ntdll/unix/thread.c
>>>> @@ -29,6 +29,8 @@
>>>>    #include <errno.h>
>>>>    #include <limits.h>
>>>>    #include <stdarg.h>
>>>> +#include <stdio.h>
>>>> +#include <string.h>
>>>>    #include <pthread.h>
>>>>    #include <signal.h>
>>>>    #include <sys/types.h>
>>>> @@ -821,6 +823,59 @@ static void wow64_context_to_server( context_t
>>>> *to, const WOW64_CONTEXT *from )
>>>>      #endif /* __x86_64__ */
>>>>    +#ifdef linux
>>>> +static BOOL get_thread_times(int unix_pid, int unix_tid,
>>>> LARGE_INTEGER *kernel_time, LARGE_INTEGER *user_time)
>>>> +{
>>>> +    unsigned long clocks_per_sec = sysconf( _SC_CLK_TCK );
>>>> +    unsigned long usr, sys;
>>>> +    const char *pos;
>>>> +    char buf[512];
>>>> +    FILE *f;
>>>> +    int i;
>>>> +
>>>> +    sprintf( buf, "/proc/%u/task/%u/stat", unix_pid, unix_tid );
>>>> +    if (!(f = fopen( buf, "r" )))
>>>> +    {
>>>> +        ERR("Failed to open %s: %s\n", buf, strerror(errno));
>>>> +        return FALSE;
>>>> +    }
>>>> +
>>>> +    pos = fgets( buf, sizeof(buf), f );
>>>> +    fclose( f );
>>>> +
>>>> +    /* the process name is printed unescaped, so we have to skip to
>>>> the last ')'
>>>> +     * to avoid misinterpreting the string */
>>>> +    if (pos) pos = strrchr( pos, ')' );
>>>> +    if (pos) pos = strchr( pos + 1, ' ' );
>>>> +    if (pos) pos++;
>>>> +
>>>> +    /* skip over the following fields: state, ppid, pgid, sid,
>>>> tty_nr, tty_pgrp,
>>>> +     * task->flags, min_flt, cmin_flt, maj_flt, cmaj_flt */
>>>> +    for (i = 0; i < 11 && pos; i++)
>>>> +    {
>>>> +        pos = strchr( pos + 1, ' ' );
>>>> +        if (pos) pos++;
>>>> +    }
>>>> +
>>>> +    /* the next two values are user and system time */
>>>> +    if (pos && (sscanf( pos, "%lu %lu", &usr, &sys ) == 2))
>>>> +    {
>>>> +        kernel_time->QuadPart = (ULONGLONG)sys * 10000000 /
>>>> clocks_per_sec;
>>>> +        user_time->QuadPart = (ULONGLONG)usr * 10000000 /
>>>> clocks_per_sec;
>>>> +        return TRUE;
>>>> +    }
>>>> +
>>>> +    ERR("Failed to parse %s\n", debugstr_a(buf));
>>>> +    return FALSE;
>>>> +}
>>>> +#else
>>>> +static BOOL get_thread_times(int unix_pid, int unix_tid,
>>>> LARGE_INTEGER *kernel_time, LARGE_INTEGER *user_time)
>>>> +{
>>>> +    static int once;
>>>> +    if (!once++) FIXME("not implemented on this platform\n");
>>>> +    return FALSE;
>>>> +}
>>>> +#endif
>>>>    
>>>> /******************************************************************************
>>>>
>>>>
>>>>     *              NtQueryInformationThread  (NTDLL.@)
>>>> @@ -886,6 +941,7 @@ NTSTATUS WINAPI NtQueryInformationThread( HANDLE
>>>> handle, THREADINFOCLASS class,
>>>>        case ThreadTimes:
>>>>        {
>>>>            KERNEL_USER_TIMES kusrt;
>>>> +        int unix_pid, unix_tid;
>>>>              SERVER_START_REQ( get_thread_times )
>>>>            {
>>>> @@ -895,15 +951,21 @@ NTSTATUS WINAPI NtQueryInformationThread( HANDLE
>>>> handle, THREADINFOCLASS class,
>>>>                {
>>>>                    kusrt.CreateTime.QuadPart = reply->creation_time;
>>>>                    kusrt.ExitTime.QuadPart = reply->exit_time;
>>>> +                unix_pid = reply->unix_pid;
>>>> +                unix_tid = reply->unix_tid;
>>>>                }
>>>>            }
>>>>            SERVER_END_REQ;
>>>>            if (status == STATUS_SUCCESS)
>>>>            {
>>>> -            /* We call times(2) for kernel time or user time */
>>>> -            /* We can only (portably) do this for the current
>>>> thread */
>>>> -            if (handle == GetCurrentThread())
>>>> +            BOOL ret = FALSE;
>>>> +
>>>> +            kusrt.KernelTime.QuadPart = kusrt.UserTime.QuadPart = 0;
>>>> +            if (unix_pid != -1 && unix_tid != -1)
>>>> +                ret = get_thread_times( unix_pid, unix_tid,
>>>> &kusrt.KernelTime, &kusrt.UserTime );
>>>> +            if (!ret && handle == GetCurrentThread())
>>>>                {
>>>> +                /* fall back to process times */
>>>>                    struct tms time_buf;
>>>>                    long clocks_per_sec = sysconf(_SC_CLK_TCK);
>>>>    @@ -911,20 +973,6 @@ NTSTATUS WINAPI NtQueryInformationThread(
>>>> HANDLE handle, THREADINFOCLASS class,
>>>>                    kusrt.KernelTime.QuadPart =
>>>> (ULONGLONG)time_buf.tms_stime * 10000000 / clocks_per_sec;
>>>>                    kusrt.UserTime.QuadPart =
>>>> (ULONGLONG)time_buf.tms_utime * 10000000 / clocks_per_sec;
>>>>                }
>>>> -            else
>>>> -            {
>>>> -                static BOOL reported = FALSE;
>>>> -
>>>> -                kusrt.KernelTime.QuadPart = 0;
>>>> -                kusrt.UserTime.QuadPart = 0;
>>>> -                if (reported)
>>>> -                    TRACE("Cannot get kerneltime or usertime of other
>>>> threads\n");
>>>> -                else
>>>> -                {
>>>> -                    FIXME("Cannot get kerneltime or usertime of other
>>>> threads\n");
>>>> -                    reported = TRUE;
>>>> -                }
>>>> -            }
>>>>                if (data) memcpy( data, &kusrt, min( length,
>>>> sizeof(kusrt) ));
>>>>                if (ret_len) *ret_len = min( length, sizeof(kusrt) );
>>>>            }
>>>> diff --git a/server/protocol.def b/server/protocol.def
>>>> index c3442c06e9..54668a8cdc 100644
>>>> --- a/server/protocol.def
>>>> +++ b/server/protocol.def
>>>> @@ -967,6 +967,8 @@ struct rawinput_device
>>>>    @REPLY
>>>>        timeout_t    creation_time; /* thread creation time */
>>>>        timeout_t    exit_time;     /* thread exit time */
>>>> +    int          unix_pid;      /* thread native pid */
>>>> +    int          unix_tid;      /* thread native pid */
>>>>    @END
>>>>      diff --git a/server/thread.c b/server/thread.c
>>>> index e2bfa50c7b..3cf447b1a0 100644
>>>> --- a/server/thread.c
>>>> +++ b/server/thread.c
>>>> @@ -1554,6 +1554,8 @@ DECL_HANDLER(get_thread_times)
>>>>        {
>>>>            reply->creation_time  = thread->creation_time;
>>>>            reply->exit_time      = thread->exit_time;
>>>> +        reply->unix_pid       = thread->unix_pid;
>>>> +        reply->unix_tid       = thread->unix_tid;
>>>>              release_object( thread );
>>>>        }
>>>>
>>>
>>> Wouldn't it be possible (and portable) to track these times in
>>> wineserver?
>>
>> I'm not sure I understand; how would that be more portable?
>>
> 
> Of course it wouldn't track Linux kernel time, but instead time spent in
> wineserver as kernel time, but that's what wineserver is supposed to be
> right?

That could arguably make sense, though in that case I think we'd want to
track the time spent in unix libraries in general, not just in the server.

I'll leave it to the maintainer to decide which implementation is more
correct; I have no idea myself.

> 
>>>
>>> Also FWIW these requests are used a lot by anti-debug checks, and
>>> although they are apparently happy with the times not being reported,
>>> I'm a little bit that reporting times could break some of them (although
>>> I haven't checked yet if they did, I'll try a few ones).
>>
>> For what it's worth, this patch has been in wine-staging for a long time
>> (not that that necessarily means anything, of course).
>> -- 
> Rémi Bernon <rbernon at codeweavers.com>
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200629/e55eb649/attachment-0001.sig>


More information about the wine-devel mailing list