[PATCH 0/1] MR5353: ddraw: Allow table to grow

[David Kahurani (＠ReDress)]

This table doesn't really grow when the total number of handles
exceeds the initial size. It will instead try to wrap and lead to
an access violation.

Signed-off-by: David Kahurani <k.kahurani(a)gmail.com>

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

[PATCH 1/1] ddraw: Allow table to grow

[David Kahurani]

From: David Kahurani <k.kahurani(a)gmail.com>

This table doesn't really grow when the total number of handles
exceeds the initial size. It will instead try to wrap and lead to
an access violation.

Signed-off-by: David Kahurani <k.kahurani(a)gmail.com>
---
 dlls/ddraw/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dlls/ddraw/main.c b/dlls/ddraw/main.c
index 1ba2900704f..4dec3293acb 100644
--- a/dlls/ddraw/main.c
+++ b/dlls/ddraw/main.c
@@ -150,6 +150,7 @@ DWORD ddraw_allocate_handle(struct ddraw_handle_table *t, void *object, enum ddr
         t->free_entries = entry->object;
         entry->object = object;
         entry->type = type;
+        t->entry_count++;
 
         return idx;
     }
@@ -198,6 +199,7 @@ void *ddraw_free_handle(struct ddraw_handle_table *t, DWORD handle, enum ddraw_h
     entry->object = t->free_entries;
     entry->type = DDRAW_HANDLE_FREE;
     t->free_entries = entry;
+    t->entry_count--;
 
     return object;
 }
-- 
GitLab

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

[PATCH 1/1] ddraw: Allow table to grow

[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=144189

Your paranoid android.


=== debian11 (32 bit report) ===

ddraw:
Unhandled exception: page fault on read access to 0x00000128 in 32-bit code (0x78e1ef92).
Unhandled exception: page fault on read access to 0x00000128 in 32-bit code (0x78e1ef92).

=== debian11b (64 bit WoW report) ===

ddraw:
Unhandled exception: page fault on read access to 0x0000000000000174 in 64-bit code (0x0000007905dcbb).
Unhandled exception: page fault on read access to 0x0000000000000174 in 64-bit code (0x0000007905dcbb).

[Nikolay Sivov (＠nsivov)]

This does not look right. Do you see the problem you described with some application? This table is 64 entries initially, once 64 were allocated, it will grow to 64 + 64 / 2 size. When entries are freed they are chained in a free list, which is checked first on next allocation.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5353#note_65353

[David Kahurani (＠ReDress)]

What do you suppose is right? Because what is there doesn't look right either.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5353#note_65364

[Henri Verbeet (＠hverbeet)]

> What do you suppose is right? Because what is there doesn't look right either.

Perhaps, but I don't think the patch does a great job of explaining the issue.

In any case, note that the number of entries shouldn't change when an existing entry is marked as free in ddraw_free_handle(); the entry doesn't go away, only its type is changed to DDRAW_HANDLE_FREE. Similarly, reusing an existing entry from the free list doesn't change the total number of allocated entries.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5353#note_65370

[Elizabeth Figura (＠zfigura)]

What exactly is the bug here? Can we have a test or a reference to an application reproducing it?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5353#note_67845

[Elizabeth Figura (＠zfigura)]

@ReDress, ping?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5353#note_72155

[David Kahurani (＠ReDress)]

Uhmm... sorry gmail has just been clobbering all email from gitlab for a while until recently. I didn't really see this in time but it seems they're not doing that anymore.

I can't seem to wrap my head around how this table is growing and if it is growing correctly. However, since no-one else seems to be in doubt, I will just close this, at least for now.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5353#note_72192

[PATCH 0/1] MR5353: ddraw: Allow table to grow - closed

[David Kahurani (＠ReDress)]

This merge request was closed by David Kahurani.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5353