July 2016 - Wine-Devel - list.winehq.org

Re: [PATCH 1/6] d3drm: Implement IDirect3DRMViewport*::Init. (v7)

by Henri Verbeet

On 27 July 2016 at 21:09, Aaryaman Vasishta <jem456.vasishta(a)gmail.com> wrote: > On Thu, Jul 28, 2016 at 12:13 AM, Stefan Dösinger >> > + if (material) >> > + IDirect3DMaterial_Release(material); >> This will destroy the material, viewport::SetBackground doesn't addref it. > > Right, henri suggested the same. I'll move this outside the cleanup label. I did say the sequence was odd, but it does work because "material" is set to NULL on successful viewport initialisation. On the other hand, I just noticed you'll also leak the material if IDirect3DViewport_SetBackground() fails.

July 27, 2016

Re: [PATCH 3/6] d3drm: Implement IDirect3DRMViewport::GetDirect3DViewport. (resend)

by Stefan Dösinger

On 2016-07-27 20:08, Stefan Dösinger wrote: > You have some lines > 120 characters in here. Otherwise looks good to me. Actually, I accidentally counted the + from the patch, so it's exactly 120.

July 27, 2016

Re: [PATCH 1/6] d3drm: Implement IDirect3DRMViewport*::Init. (v7)

by Aaryaman Vasishta

On Thu, Jul 28, 2016 at 12:13 AM, Stefan Dösinger <stefandoesinger(a)gmail.com > wrote: > On 2016-07-19 19:08, Aaryaman Vasishta wrote: > > + memset(&mat, 0, sizeof(mat)); > > + mat.dwSize = sizeof(mat); > > + mat.diffuse.r = RGBA_GETRED(color) / 255.0f; > > + mat.diffuse.g = RGBA_GETGREEN(color) / 255.0f; > > + mat.diffuse.b = RGBA_GETBLUE(color) / 255.0f; > > + mat.diffuse.a = RGBA_GETALPHA(color) / 255.0f; > Doing this here seems questionable. What happens if the camera's > background is changed after the viewport is created? You'll probably want > to update the material property in viewport::clear(), or maybe in > camera::SetSceneBackground. > Right, that'll be added in the implementation of Clear. The camera frame itself doesn't contain a reference to the viewport so I doubt SetSceneBackground has any effect on it. (See the tests I wrote for Clear). > > > + if (material) > > + IDirect3DMaterial_Release(material); > This will destroy the material, viewport::SetBackground doesn't addref it. > Right, henri suggested the same. I'll move this outside the cleanup label. > > > + IDirect3DRMDevice3_Release(device3); > > + IDirect3DRMDevice_Release(device1); > > + ref4 = get_refcount((IUnknown *)d3drm1); > > + todo_wine ok(ref4 > initial_ref1, "Expected ref4 > initial_ref1, > got initial_ref1 = %u, ref4 = %u.\n", initial_ref1, ref4); > The ref4 > inital_ref1 check isn't all too surprising because frame3 still > holds a reference to its d3drm parent. Though it seems the leaked reference > added by the second Viewport::Init call gets released somewhere. The test > doesn't exactly show where. It is possible that releasing the device frees > it, or releasing the frame does it. > The leaked references created by the second Init call are all released when the device is destroyed. This suggests that the device is using some sort of destroy callback and keeping track of all leaked references. See the ref4 == initial_ref1 test below it. I'll resend the patch with the fixes you mentioned. Thanks for the review! Cheers, Aaryaman

July 27, 2016

Re: [PATCH 3/6] d3drm: Implement IDirect3DRMViewport::GetDirect3DViewport. (resend)

by Stefan Dösinger

You have some lines > 120 characters in here. Otherwise looks good to me. On 2016-07-19 19:08, Aaryaman Vasishta wrote: > Signed-off-by: Aaryaman Vasishta <jem456.vasishta(a)gmail.com> > --- > dlls/d3drm/tests/d3drm.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++- > dlls/d3drm/viewport.c | 18 ++++++++++--- > 2 files changed, 80 insertions(+), 5 deletions(-) > > diff --git a/dlls/d3drm/tests/d3drm.c b/dlls/d3drm/tests/d3drm.c > index 17cf2dd..fc0b6b4 100644 > --- a/dlls/d3drm/tests/d3drm.c > +++ b/dlls/d3drm/tests/d3drm.c > @@ -1474,12 +1474,13 @@ static void test_Viewport(void) > IDirect3DRMFrame3 *frame3; > IDirect3DRMViewport *viewport; > IDirect3DRMViewport2 *viewport2; > + IDirect3DViewport *d3d_viewport; > IDirect3DRMObject *obj, *obj2; > GUID driver; > HWND window; > RECT rc; > DWORD size, data, ref1, ref2, ref3, ref4; > - DWORD initial_ref1, initial_ref2, initial_ref3, device_ref, frame_ref, frame_ref2; > + DWORD initial_ref1, initial_ref2, initial_ref3, device_ref, frame_ref, frame_ref2, viewport_ref; > CHAR cname[64] = {0}; > > window = CreateWindowA("static", "d3drm_test", WS_OVERLAPPEDWINDOW, 0, 0, 300, 200, 0, 0, 0, 0); > @@ -1641,13 +1642,47 @@ static void test_Viewport(void) > > hr = IDirect3DRM2_CreateViewport(d3drm2, device1, frame, rc.left, rc.top, rc.right, rc.bottom, &viewport); > ok(hr == D3DRM_OK, "Cannot get IDirect3DRMViewport interface (hr = %x)\n", hr); > + hr = IDirect3DRMViewport_GetDirect3DViewport(viewport, &d3d_viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DViewport interface (hr = %#x).\n", hr); > + viewport_ref = get_refcount((IUnknown *)d3d_viewport); > + hr = IDirect3DRMViewport_GetDirect3DViewport(viewport, &d3d_viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DViewport interface (hr = %#x).\n", hr); > + ref4 = get_refcount((IUnknown *)d3d_viewport); > + ok(ref4 > viewport_ref, "Expected ref4 > viewport_ref, got ref4 = %u, viewport_ref = %u.\n", ref4, viewport_ref); > + IDirect3DViewport_Release(d3d_viewport); > + ref4 = get_refcount((IUnknown *)d3d_viewport); > + ok(ref4 == viewport_ref, "Expected ref4 == viewport_ref, got ref4 = %u, viewport_ref = %u.\n", ref4, viewport_ref); > + IDirect3DViewport_Release(d3d_viewport); > IDirect3DRMViewport_Release(viewport); > + > hr = IDirect3DRM3_CreateViewport(d3drm3, device3, frame3, rc.left, rc.top, rc.right, rc.bottom, &viewport2); > ok(hr == D3DRM_OK, "Cannot get IDirect3DRMViewport2 interface (hr = %x)\n", hr); > + hr = IDirect3DRMViewport2_GetDirect3DViewport(viewport2, &d3d_viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DViewport interface (hr = %#x).\n", hr); > + viewport_ref = get_refcount((IUnknown *)d3d_viewport); > + hr = IDirect3DRMViewport2_GetDirect3DViewport(viewport2, &d3d_viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DViewport interface (hr = %#x).\n", hr); > + ref4 = get_refcount((IUnknown *)d3d_viewport); > + ok(ref4 > viewport_ref, "Expected ref4 > viewport_ref, got ref4 = %u, viewport_ref = %u.\n", ref4, viewport_ref); > + IDirect3DViewport_Release(d3d_viewport); > + ref4 = get_refcount((IUnknown *)d3d_viewport); > + ok(ref4 == viewport_ref, "Expected ref4 == viewport_ref, got ref4 = %u, viewport_ref = %u.\n", ref4, viewport_ref); > + IDirect3DViewport_Release(d3d_viewport); > IDirect3DRMViewport2_Release(viewport2); > > hr = IDirect3DRM_CreateViewport(d3drm1, device1, frame, rc.left, rc.top, rc.right, rc.bottom, &viewport); > ok(hr == D3DRM_OK, "Cannot get IDirect3DRMViewport interface (hr = %x)\n", hr); > + hr = IDirect3DRMViewport_GetDirect3DViewport(viewport, &d3d_viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DViewport interface (hr = %#x).\n", hr); > + viewport_ref = get_refcount((IUnknown *)d3d_viewport); > + hr = IDirect3DRMViewport_GetDirect3DViewport(viewport, &d3d_viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DViewport interface (hr = %#x).\n", hr); > + ref4 = get_refcount((IUnknown *)d3d_viewport); > + ok(ref4 > viewport_ref, "Expected ref4 > viewport_ref, got ref4 = %u, viewport_ref = %u.\n", ref4, viewport_ref); > + IDirect3DViewport_Release(d3d_viewport); > + ref4 = get_refcount((IUnknown *)d3d_viewport); > + ok(ref4 == viewport_ref, "Expected ref4 == viewport_ref, got ref4 = %u, viewport_ref = %u.\n", ref4, viewport_ref); > + IDirect3DViewport_Release(d3d_viewport); > > hr = IDirect3DRMViewport_QueryInterface(viewport, &IID_IDirect3DRMObject, (void**)&obj); > ok(hr == D3DRM_OK, "expected D3DRM_OK (hr = %x)\n", hr); > @@ -1713,6 +1748,9 @@ static void test_Viewport(void) > ref4 = get_refcount((IUnknown *)d3drm3); > ok(ref4 == ref3, "Expected ref4 == ref3, got ref3 = %u, ref4 = %u.\n", ref3, ref4); > > + hr = IDirect3DRMViewport_GetDirect3DViewport(viewport, &d3d_viewport); > + ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr); > + > /* Test all failures together */ > hr = IDirect3DRMViewport_Init(viewport, NULL, frame, rc.left, rc.top, rc.right, rc.bottom); > ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr); > @@ -1740,6 +1778,18 @@ static void test_Viewport(void) > ref4 = get_refcount((IUnknown *)frame); > ok(ref4 > frame_ref, "Expected ref4 > frame_ref, got frame_ref = %u, ref4 = %u.\n", frame_ref, ref4); > > + hr = IDirect3DRMViewport_GetDirect3DViewport(viewport, &d3d_viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DViewport interface (hr = %#x).\n", hr); > + viewport_ref = get_refcount((IUnknown *)d3d_viewport); > + hr = IDirect3DRMViewport_GetDirect3DViewport(viewport, &d3d_viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DViewport interface (hr = %#x).\n", hr); > + ref4 = get_refcount((IUnknown *)d3d_viewport); > + ok(ref4 > viewport_ref, "Expected ref4 > viewport_ref, got ref4 = %u, viewport_ref = %u.\n", ref4, viewport_ref); > + IDirect3DViewport_Release(d3d_viewport); > + ref4 = get_refcount((IUnknown *)d3d_viewport); > + ok(ref4 == viewport_ref, "Expected ref4 == viewport_ref, got ref4 = %u, viewport_ref = %u.\n", ref4, viewport_ref); > + IDirect3DViewport_Release(d3d_viewport); > + > hr = IDirect3DRMViewport_Init(viewport, device1, frame, rc.left, rc.top, rc.right, rc.bottom); > ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr); > > @@ -1768,6 +1818,9 @@ static void test_Viewport(void) > ref4 = get_refcount((IUnknown *)d3drm3); > ok(ref4 == ref3, "Expected ref4 == ref3, got ref3 = %u, ref4 = %u.\n", ref3, ref4); > > + hr = IDirect3DRMViewport2_GetDirect3DViewport(viewport2, &d3d_viewport); > + ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr); > + > hr = IDirect3DRMViewport2_Init(viewport2, NULL, frame3, rc.left, rc.top, rc.right, rc.bottom); > ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr); > hr = IDirect3DRMViewport2_Init(viewport2, device3, NULL, rc.left, rc.top, rc.right, rc.bottom); > @@ -1788,6 +1841,18 @@ static void test_Viewport(void) > ref4 = get_refcount((IUnknown *)frame3); > ok(ref4 > frame_ref2, "Expected ref4 > frame_ref2, got frame_ref2 = %u, ref4 = %u.\n", frame_ref2, ref4); > > + hr = IDirect3DRMViewport2_GetDirect3DViewport(viewport2, &d3d_viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DViewport interface (hr = %#x).\n", hr); > + viewport_ref = get_refcount((IUnknown *)d3d_viewport); > + hr = IDirect3DRMViewport2_GetDirect3DViewport(viewport2, &d3d_viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DViewport interface (hr = %#x).\n", hr); > + ref4 = get_refcount((IUnknown *)d3d_viewport); > + ok(ref4 > viewport_ref, "Expected ref4 > viewport_ref, got ref4 = %u, viewport_ref = %u.\n", ref4, viewport_ref); > + IDirect3DViewport_Release(d3d_viewport); > + ref4 = get_refcount((IUnknown *)d3d_viewport); > + ok(ref4 == viewport_ref, "Expected ref4 == viewport_ref, got ref4 = %u, viewport_ref = %u.\n", ref4, viewport_ref); > + IDirect3DViewport_Release(d3d_viewport); > + > hr = IDirect3DRMViewport2_Init(viewport2, device3, frame3, rc.left, rc.top, rc.right, rc.bottom); > ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr); > > diff --git a/dlls/d3drm/viewport.c b/dlls/d3drm/viewport.c > index 730a1f5..09b0e83 100644 > --- a/dlls/d3drm/viewport.c > +++ b/dlls/d3drm/viewport.c > @@ -826,17 +826,27 @@ static D3DRMPROJECTIONTYPE WINAPI d3drm_viewport1_GetProjection(IDirect3DRMViewp > static HRESULT WINAPI d3drm_viewport2_GetDirect3DViewport(IDirect3DRMViewport2 *iface, > IDirect3DViewport **viewport) > { > - FIXME("iface %p, viewport %p stub!\n", iface, viewport); > + struct d3drm_viewport *viewport_object = impl_from_IDirect3DRMViewport2(iface); > > - return E_NOTIMPL; > + TRACE("iface %p, viewport %p.\n", iface, viewport); > + > + if (!viewport_object->d3d_viewport) > + return D3DRMERR_BADOBJECT; > + > + *viewport = viewport_object->d3d_viewport; > + IDirect3DViewport_AddRef(*viewport); > + > + return D3DRM_OK; > } > > static HRESULT WINAPI d3drm_viewport1_GetDirect3DViewport(IDirect3DRMViewport *iface, > IDirect3DViewport **viewport) > { > - FIXME("iface %p, viewport %p stub!\n", iface, viewport); > + struct d3drm_viewport *viewport_object = impl_from_IDirect3DRMViewport(iface); > > - return E_NOTIMPL; > + TRACE("iface %p, viewport %p.\n", iface, viewport); > + > + return d3drm_viewport2_GetDirect3DViewport(&viewport_object->IDirect3DRMViewport2_iface, viewport); > } > > static HRESULT WINAPI d3drm_viewport2_TransformVectors(IDirect3DRMViewport2 *iface, >

July 27, 2016

Re: [PATCH 3/3] ntdll: For Mac 64-bit, poke NtCurrentTeb()->ThreadLocalStoragePointer to the corresponding offset from %gs.

by Sebastian Lackner

On 27.07.2016 20:25, Ken Thomases wrote: > On Jul 27, 2016, at 12:53 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: >> >> I have only two suggestions: >> - gsbase_offset == 0 (key == i) could also be valid, so it would be useful to distinguish this case. > > It can't be valid because that would put the TLS at the beginning of struct _pthread, but the ABI actually reveals the first couple of fields. pthread_t is a typedef for __darwin_pthread_t which is a typedef for struct _opaque_pthread_t which is defined (not just declared) in the headers (/usr/include/sys/_pthread/_pthread_types.h in the 10.11 SDK) and is less opaque than its name would suggest. Comments in the definition of struct _pthread in libpthread make note of the ABI compatibility factor. > > In practice, the TLS array is at the end of the struct and I think Apple told us it has to be there. When you are sure that this is always true you can start searching at (key + 1). > >> - the logic could be moved to a helper function, for example using the init once mechanism > > I suppose, but it will be called for the first thread before there's a chance of creating secondary threads. So, there's no race. Also, I haven't looked, but I'm not sure what thread setup has to have been completed before RtlRunOnceExecuteOnce() can safely be used. It looks like RtlRunOnceExecuteOnce is not safe to use (keyed_event not initialized yet). You could use pthread (like used before in init_teb_key), but probably moving to a function is sufficient to improve code readability. By avoiding the heavily indented code it would already look a bit less hacky. ;) > > -Ken >

July 27, 2016

Re: [PATCH 2/6] d3drm: Implement IDirect3DRM*::CreateViewport. (v2 resend)

by Stefan Dösinger

On 2016-07-19 19:08, Aaryaman Vasishta wrote: > + if (FAILED(hr = IDirect3DRMDevice_QueryInterface(device, &IID_IDirect3DRMDevice3, (void **)&device3))) > + return hr; > + > + if (FAILED(hr = IDirect3DRMFrame_QueryInterface(camera, &IID_IDirect3DRMFrame3, (void **)&camera3))) > + { > + IDirect3DRMDevice3_Release(device3); > + return hr; > + } > + IDirect3DRMDevice_Release(device); > + IDirect3DRMFrame_Release(camera); This releases references you don't own. Same in the V2 -> V3 thunk. > + if (!device || !camera) > + return D3DRMERR_BADOBJECT; > + if (!viewport) > + return D3DRMERR_BADVALUE; These checks are redundant, Init() does that as well. > + ok(ref4 == frame_ref, "Expected ref4 == frame_ref, got frame_ref = %u, ref4 = %u.\n", frame_ref, ref4); > + > + device_ref = get_refcount((IUnknown *)device1); > + frame_ref = get_refcount((IUnknown *)frame); The get_refcount here seems redundant because you've just shown that it is back to where it used to be.

July 27, 2016

Re: [PATCH 1/6] d3drm: Implement IDirect3DRMViewport*::Init. (v7)

by Stefan Dösinger

On 2016-07-19 19:08, Aaryaman Vasishta wrote: > + memset(&mat, 0, sizeof(mat)); > + mat.dwSize = sizeof(mat); > + mat.diffuse.r = RGBA_GETRED(color) / 255.0f; > + mat.diffuse.g = RGBA_GETGREEN(color) / 255.0f; > + mat.diffuse.b = RGBA_GETBLUE(color) / 255.0f; > + mat.diffuse.a = RGBA_GETALPHA(color) / 255.0f; Doing this here seems questionable. What happens if the camera's background is changed after the viewport is created? You'll probably want to update the material property in viewport::clear(), or maybe in camera::SetSceneBackground. > + if (material) > + IDirect3DMaterial_Release(material); This will destroy the material, viewport::SetBackground doesn't addref it. > + hr = IDirect3DRMViewport_Init(viewport, device1, frame, rc.left, rc.top, rc.right + 1, rc.bottom + 1); > + ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr); > + hr = IDirect3DRMViewport_Init(viewport, device1, frame, rc.left, rc.top, rc.right + 1, rc.bottom); > + ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr); > + hr = IDirect3DRMViewport_Init(viewport, device1, frame, rc.left, rc.top, rc.right + 1, rc.bottom + 1); > + ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr); The first and third test are identical. > + IDirect3DRMDevice3_Release(device3); > + IDirect3DRMDevice_Release(device1); > + ref4 = get_refcount((IUnknown *)d3drm1); > + todo_wine ok(ref4 > initial_ref1, "Expected ref4 > initial_ref1, got initial_ref1 = %u, ref4 = %u.\n", initial_ref1, ref4); The ref4 > inital_ref1 check isn't all too surprising because frame3 still holds a reference to its d3drm parent. Though it seems the leaked reference added by the second Viewport::Init call gets released somewhere. The test doesn't exactly show where. It is possible that releasing the device frees it, or releasing the frame does it.

July 27, 2016

Re: [PATCH 3/3] ntdll: For Mac 64-bit, poke NtCurrentTeb()->ThreadLocalStoragePointer to the corresponding offset from %gs.

by Ken Thomases

On Jul 27, 2016, at 12:53 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > > I have only two suggestions: > - gsbase_offset == 0 (key == i) could also be valid, so it would be useful to distinguish this case. It can't be valid because that would put the TLS at the beginning of struct _pthread, but the ABI actually reveals the first couple of fields. pthread_t is a typedef for __darwin_pthread_t which is a typedef for struct _opaque_pthread_t which is defined (not just declared) in the headers (/usr/include/sys/_pthread/_pthread_types.h in the 10.11 SDK) and is less opaque than its name would suggest. Comments in the definition of struct _pthread in libpthread make note of the ABI compatibility factor. In practice, the TLS array is at the end of the struct and I think Apple told us it has to be there. > - the logic could be moved to a helper function, for example using the init once mechanism I suppose, but it will be called for the first thread before there's a chance of creating secondary threads. So, there's no race. Also, I haven't looked, but I'm not sure what thread setup has to have been completed before RtlRunOnceExecuteOnce() can safely be used. -Ken

July 27, 2016

Re: [PATCH 3/3] ntdll: For Mac 64-bit, poke NtCurrentTeb()->ThreadLocalStoragePointer to the corresponding offset from %gs.

by Sebastian Lackner

On 27.07.2016 19:42, Ken Thomases wrote: >> >> Hm, I see. Do you know if the offset really changed in the past? > > Yes, it has changed in the past. > >> I am asking >> because I assume the bruteforcing is an attempt to make the code more reliable, >> however on the other hand you still make the assumption that pthread keys >> directly correspond to indices into the %gs segment, which could also change. > > Well, we made various proposals to Apple to get a more comprehensive, less kludgy fix and they were rejected because of binary compatibility. Basically, they were unwilling to change how pthread_getspecific() works. Mono depends on it, for example. So, it seems pretty reliable. > >> A check that it matches expected behavior is probably sufficient. >> >> Besides that, if the gsbase cannot be located, it probably would be preferred >> to skip this code on following attempts. > > It already does that. gsbase_offset is static. It will only be negative on the first call. Sorry, I missed the "gsbase_offset = 0;" call. You are probably right that there is not much to improve unless Apple offers a better interface. I have only two suggestions: - gsbase_offset == 0 (key == i) could also be valid, so it would be useful to distinguish this case. - the logic could be moved to a helper function, for example using the init once mechanism > > -Ken >

July 27, 2016

Re: [PATCH 3/3] ntdll: For Mac 64-bit, poke NtCurrentTeb()->ThreadLocalStoragePointer to the corresponding offset from %gs.

by Ken Thomases

On Jul 27, 2016, at 12:14 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > > On 27.07.2016 18:27, Ken Thomases wrote: >> On Jul 27, 2016, at 11:06 AM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: >>> >>> On 27.07.2016 17:42, Ken Thomases wrote: >>>> 64-bit Windows apps have hard-coded accesses to %gs:0x58 baked into them. They >>>> need to find the ThreadLocalStoragePointer there. >>>> >>>> Technically, the gsbase register and the memory it points to belong to the >>>> pthread implementation on macOS. It's used for the pthread TLS implementation. >>>> Slot 11 (offset 0x58) is currently used for the implementation of the ttyname() >>>> system library function. We do not anticipate that Wine or any of the system >>>> libraries or frameworks it uses will call ttyname(). Furthermore, Apple has >>>> made it so that future releases of macOS will no longer use that slot. So, we >>>> hijack it for our purposes. >>>> >>>> Signed-off-by: Ken Thomases <ken(a)codeweavers.com> >>>> --- >>>> dlls/ntdll/loader.c | 11 +++++++- >>>> dlls/ntdll/signal_x86_64.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 72 insertions(+), 1 deletion(-) >>> >>> I'm not sure if I correctly understand the purpose of this patch. If you have to >>> set %gs:0x58 to some specific value, why not just use an assembly instruction for that? >>> >> >> alloc_tls_slot() is setting ThreadLocalStoragePointer for all threads, not just the current one. >> >> -Ken >> > > Hm, I see. Do you know if the offset really changed in the past? Yes, it has changed in the past. > I am asking > because I assume the bruteforcing is an attempt to make the code more reliable, > however on the other hand you still make the assumption that pthread keys > directly correspond to indices into the %gs segment, which could also change. Well, we made various proposals to Apple to get a more comprehensive, less kludgy fix and they were rejected because of binary compatibility. Basically, they were unwilling to change how pthread_getspecific() works. Mono depends on it, for example. So, it seems pretty reliable. > A check that it matches expected behavior is probably sufficient. > > Besides that, if the gsbase cannot be located, it probably would be preferred > to skip this code on following attempts. It already does that. gsbase_offset is static. It will only be negative on the first call. -Ken

July 27, 2016