31 Aug
2016
31 Aug
'16
9:12 a.m.
TL;WR: I'd like to share a struct definition between ntdll and
kernel32, passed through the PEB. Is this kosher according to Wine
style/design, and if so, which header should I stick it in?
--
I'd like to have fiber-local storage callbacks in Wine.
http://source.winehq.org/git/wine.git/commit/556fef3d
http://source.winehq.org/git/wine.git/commit/ab11a5ae
http://source.winehq.org/git/wine.git/commit/4e8f43f4
So, after writing the conformance tests, it's time to implement the feature :-)
I've got the feature all done, except for one issue. The proximate
issue is that when NtTerminateProcess is used on Windows, FLS
callbacks are not called for threads that are aborted (and those
threads do not get a DLL_THREAD_DETACH notification). However, if
during DLL_PROCESS_DETACH the FLS slot is then freed, then the
callbacks are called on behalf of those (already-terminated) threads
by the process-detaching thread. The behavior appears to be thus: if
the thread is exited cleanly (through LdrShutdownThread), then the FLS
callbacks are called for that thread; otherwise on an unclean exit the
FLS slots for the thread are transferred to some global list, and the
callbacks for that list are called when FlsFree is later called.
The underlying problem regards doing that transfer to the global list.
LdrShutdownThread and its other friends reside in ntdll, so ntdll will
have to know how to manage FLS. The FLS API however resides in
kernel32, so kernel32 will have to know, as well. For this purpose
I'd like to pass a common struct between ntdll and kernel32 through
PEB.FlsListHead. What I see though is that include/winternl.h
contains (almost all) publicly Microsoft-documented struct
definitions, and I don't see anywhere else where such common structs
are presently shared between two DLLs. So is it the case that the
convention in Wine is to not do such things as sharing structs, or is
this also done elsewhere and I just need to find the right header for
it?
Thanks,
-John Sheu
31 Aug
31 Aug
7:21 p.m.
On Wed, Aug 31, 2016 at 02:12:23AM -0700, John Sheu wrote:
TL;WR: I'd like to share a struct definition
between ntdll and
kernel32, passed through the PEB. Is this kosher according to Wine
style/design, and if so, which header should I stick it in?
I don't know anything about the particular issue you're working on, so
I can just speak generally.
We try to avoid creating internal, Wine-only APIs. Sometimes it's
necessary, and when we do that, the header goes in <include/wine/>.
One example is "gdi_driver.h" which defines the interface between e.g.
user32 and the graphics drivers.
Are the structures you need declared publicly in the win32 API, even
as opaque pointers? If so, they may belong in <include/winternl.h>.
A specific proposal about what changes you'd like might bring more
detailed responses.
Hope that was at least a little helpful,
Andrew
10:31 p.m.
On Wed, Aug 31, 2016 at 12:21 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote:
On Wed, Aug 31, 2016 at 02:12:23AM -0700, John Sheu
wrote:
Thanks Andrew!
A little more elaboration, on what I already wrote:
ntdll and kernel32 will have to jointly manage ownership of the
storage array that FLS data is stored in, because it can be deleted
either
1. When a thread is shutdown, in the LdrShutdownThread path.
2. When a fiber is deleted, in when DeleteFiber is called.
In addition, (3) if the thread is aborted (through
NtTerminateProcess), the storage array isn't deleted on thread abort,
since (behavior confirmed by conformance tests) the FLS data is used
later when FlsFree is called.
So we have cases where in (1) ntdll deletes the FLS data array, (2)
kernel32 deletes the FLS data array, and (3) ntdll passes ownership of
the data array to kernel32. Thus both dlls will have to have to
jointly manage ownership of the array.
--
The question is how we manage this. One option is to introduce
additional function call entry points to either ntdll or kernel32,
called by the other, to manages FLS lifetime. I see though that the
list of entry points is pretty tightly defined in the .spec files for
each DLL, and the convention may be (correct me if I'm wrong) not to
introduce "weird" private Wine-only APIs to the list. The other
option is to use some internal structs, pointed to by the PEB and TEB
structures. That's the approach I'd like to go with.
My proposal would be:
1. Define a common struct to hold the FLS data array, that would be:
struct FlsSlots {
LIST_ENTRY fls_link;
void fls_slots[SLOT_COUNT];
};
2. TEB.FlsSlots right now is a void** member; make this a struct
FlsSlots* member instead.
3. Link the FlsSlots struct into PEB.FlsListHead.
This would require that struct FlsSlots be defined somewhere -- either
in wine/include, as mentioned, or in <include/winternl.h>. Since it
isn't declared publicly in the win32 API, even as an opaque pointer --
I'm leaning toward the former. Maybe in "wine/include/fls.h" ?
Thanks,
-John Sheu
TL;WR: I'd like to share a struct definition
between ntdll and
kernel32, passed through the PEB. Is this kosher according to Wine
style/design, and if so, which header should I stick it in?
I don't know anything about the particular issue you're working on, so
I can just speak generally.
We try to avoid creating internal, Wine-only APIs. Sometimes it's
necessary, and when we do that, the header goes in <include/wine/>.
One example is "gdi_driver.h" which defines the interface between e.g.
user32 and the graphics drivers.
Are the structures you need declared publicly in the win32 API, even
as opaque pointers? If so, they may belong in <include/winternl.h>.
A specific proposal about what changes you'd like might bring more
detailed responses. 
2 Sep
2 Sep
3:08 a.m.
John Sheu <sheu(a)google.com> writes:
The question is how we manage this. One option is to
introduce
additional function call entry points to either ntdll or kernel32,
called by the other, to manages FLS lifetime. I see though that the
list of entry points is pretty tightly defined in the .spec files for
each DLL, and the convention may be (correct me if I'm wrong) not to
introduce "weird" private Wine-only APIs to the list. The other
option is to use some internal structs, pointed to by the PEB and TEB
structures. That's the approach I'd like to go with.
My proposal would be:
1. Define a common struct to hold the FLS data array, that would be:
struct FlsSlots {
LIST_ENTRY fls_link;
void fls_slots[SLOT_COUNT];
};
2. TEB.FlsSlots right now is a void** member; make this a struct
FlsSlots* member instead.
3. Link the FlsSlots struct into PEB.FlsListHead.
This would require that struct FlsSlots be defined somewhere -- either
in wine/include, as mentioned, or in <include/winternl.h>. Since it
isn't declared publicly in the win32 API, even as an opaque pointer --
I'm leaning toward the former. Maybe in "wine/include/fls.h" ?
In general adding private headers is reserved for things that are used
across a wide range of modules, and cannot possibly be done through the
Windows API.
If you really have to share a private structure like this, you can
duplicate the definition, but that should only be a last resort. In
general if there's no equivalent structure on Windows, it means that
it's supposed to work in a different way.
I'll note that recent Windows versions export functions like RtlFlsAlloc
and RtlFlsFree, which probably means it should all be handled inside
ntdll. Of course it would first need some tests to find out what these
functions do.
--
Alexandre Julliard
julliard(a)winehq.org
12:01 p.m.
On Thu, Sep 1, 2016 at 8:08 PM, Alexandre Julliard <julliard(a)winehq.org> wrote:
John Sheu <sheu(a)google.com> writes:
I'm enthusiastic about putting it in <include/winternl.h>, since it
isn't a known Windows structure -- I don't doubt though that there's
an equivalent structure in Windows; we just don't know about it. But
duplicating the definition is just.. ick. So I'd just like to put it
in a private header, or perhaps in the Wine-private section of
winternl.h (and that is seeming like the better solution at the
moment).
Of course the optimal solution is to implement the Windows APIs. In
ntdll I'm aware of three FLS-related functions: RtlFlsAlloc,
RtlFlsFree, and RtlProcessFlsData. Unfortunately they are all three
undocumented, even without information on their function signatures.
So writing conformance tests for these is outside the scope my
experience (fuzz testing maybe?). If you have some advice on how such
testing is usually done for Wine, I'd be glad to hear it. Otherwise I
think I'll just go with sticking it in the #ifdef __WINESRC__ section
of winternl.h
-John Sheu
The question is how we manage this. One option
is to introduce
additional function call entry points to either ntdll or kernel32,
called by the other, to manages FLS lifetime. I see though that the
list of entry points is pretty tightly defined in the .spec files for
each DLL, and the convention may be (correct me if I'm wrong) not to
introduce "weird" private Wine-only APIs to the list. The other
option is to use some internal structs, pointed to by the PEB and TEB
structures. That's the approach I'd like to go with.
My proposal would be:
1. Define a common struct to hold the FLS data array, that would be:
struct FlsSlots {
LIST_ENTRY fls_link;
void fls_slots[SLOT_COUNT];
};
2. TEB.FlsSlots right now is a void** member; make this a struct
FlsSlots* member instead.
3. Link the FlsSlots struct into PEB.FlsListHead.
This would require that struct FlsSlots be defined somewhere -- either
in wine/include, as mentioned, or in <include/winternl.h>. Since it
isn't declared publicly in the win32 API, even as an opaque pointer --
I'm leaning toward the former. Maybe in "wine/include/fls.h" ?
In general adding private headers is reserved for things that are used
across a wide range of modules, and cannot possibly be done through the
Windows API.
If you really have to share a private structure like this, you can
duplicate the definition, but that should only be a last resort. In
general if there's no equivalent structure on Windows, it means that
it's supposed to work in a different way.
I'll note that recent Windows versions export functions like RtlFlsAlloc
and RtlFlsFree, which probably means it should all be handled inside
ntdll. Of course it would first need some tests to find out what these
functions do.
12:02 p.m.
On Fri, Sep 2, 2016 at 5:01 AM, John Sheu <sheu(a)google.com> wrote:
I'm enthusiastic about putting it in
<include/winternl.h>, since it
isn't a known Windows structure -- I don't doubt though that there's
an equivalent structure in Windows; we just don't know about it.
Correction: that should read, "I'm *not* enthusiastic about putting it
in <include/winternl.h>" ...
-John Sheu
12:27 p.m.
Am 02.09.2016 um 14:01 schrieb John Sheu:
On Thu, Sep 1, 2016 at 8:08 PM, Alexandre Julliard
<julliard(a)winehq.org> wrote:
The RtlFls* way seems the way to go...
I found some hints on the signature and more in this file (from Google? from VMWare?):
https://github.com/DynamoRIO/dynamorio/blob/master/core/win32/drwinapi/ntdl…
John Sheu <sheu(a)google.com> writes:
I'm enthusiastic about putting it in <include/winternl.h>, since it
isn't a known Windows structure -- I don't doubt though that there's
an equivalent structure in Windows; we just don't know about it. But
duplicating the definition is just.. ick. So I'd just like to put it
in a private header, or perhaps in the Wine-private section of
winternl.h (and that is seeming like the better solution at the
moment).
Of course the optimal solution is to implement the Windows APIs. In
ntdll I'm aware of three FLS-related functions: RtlFlsAlloc,
RtlFlsFree, and RtlProcessFlsData. Unfortunately they are all three
undocumented, even without information on their function signatures.
So writing conformance tests for these is outside the scope my
experience (fuzz testing maybe?). If you have some advice on how such
testing is usually done for Wine, I'd be glad to hear it. Otherwise I
think I'll just go with sticking it in the #ifdef __WINESRC__ section
of winternl.h
-John Sheu The question is how we manage this. One option
is to introduce
additional function call entry points to either ntdll or kernel32,
called by the other, to manages FLS lifetime. I see though that the
list of entry points is pretty tightly defined in the .spec files for
each DLL, and the convention may be (correct me if I'm wrong) not to
introduce "weird" private Wine-only APIs to the list. The other
option is to use some internal structs, pointed to by the PEB and TEB
structures. That's the approach I'd like to go with.
My proposal would be:
1. Define a common struct to hold the FLS data array, that would be:
struct FlsSlots {
LIST_ENTRY fls_link;
void fls_slots[SLOT_COUNT];
};
2. TEB.FlsSlots right now is a void** member; make this a struct
FlsSlots* member instead.
3. Link the FlsSlots struct into PEB.FlsListHead.
This would require that struct FlsSlots be defined somewhere -- either
in wine/include, as mentioned, or in <include/winternl.h>. Since it
isn't declared publicly in the win32 API, even as an opaque pointer --
I'm leaning toward the former. Maybe in "wine/include/fls.h" ?
In general adding private headers is reserved for things that are used
across a wide range of modules, and cannot possibly be done through the
Windows API.
If you really have to share a private structure like this, you can
duplicate the definition, but that should only be a last resort. In
general if there's no equivalent structure on Windows, it means that
it's supposed to work in a different way.
I'll note that recent Windows versions export functions like RtlFlsAlloc
and RtlFlsFree, which probably means it should all be handled inside
ntdll. Of course it would first need some tests to find out what these
functions do.
2800
days inactive
2802
days old
6 comments
4 participants
participants (4)
-
Alexandre Julliard -
Andrew Eikum -
André Hentschel -
John Sheu