[Bug 12171] New: adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

Summary: adding popupmenu to itself, then using finditem causes
                    stack overflow
           Product: Wine
           Version: 0.9.58.
          Platform: PC
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: minor
          Priority: P2
         Component: user32
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

The following code crashes WINE, but not Windows. It adds a popupmenu to the
popupmenu itself, then calls MENU_FindItem via GetMenuState. MENU_FindItem
keeps following the loop until it crashes with a stack overflow. Windows reacts
a bit strangely, too, but does not crash. Messing around in MENU_FindItem
allows WZebra (othello program, listed in the appdb) to start, but I was unable
to figure out the correct solution. Not sure if WZebra accidentally creates a
temporary loop, or that this is caused by some other regression. WZebra used to
work great a long time ago.

/* sub menu */
        hSubMenu = CreatePopupMenu();
        InsertMenu(hSubMenu, -1, MF_BYPOSITION, 1234, "loop");

/* main menu */
        hMenu = CreateMenu();
        InsertMenu(hMenu, -1, MF_BYPOSITION | MF_POPUP, (UINT)hSubMenu,
"&File");

/* add popupmenu to itself */
        ModifyMenu(hSubMenu, 1234, MF_POPUP, (UINT)hSubMenu, "loop");

/* MENU_FindItem ends up in infinite recursion */
        GetMenuState(hMenu, (UINT)hSubMenu, 0);

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #1 from Austin English   2008-03-23 22:17:33 ---
Can you compile a small program showing this?

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #2 from srepmub   2008-03-24 03:55:07 ---
Created an attachment (id=11590)
 --> (http://bugs.winehq.org/attachment.cgi?id=11590)
example crashing wine, not windows

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

Lei Zhang  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |source

--- Comment #3 from Lei Zhang   2008-03-24 15:30:28 ---
If you can, try WZebra or your test program in a few older version of Wine
(i.e. 0.9.{20,30,40} and see if it works there.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #4 from srepmub   2008-03-24 17:24:21 ---
before I do that, what's the common/usual way to locate a regression over
multiple versions? a binary search, pulling different versions out of git? any
tips?

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #5 from Lei Zhang   2008-03-24 17:28:36 ---
We have a nice wiki page on regression testing:

http://wiki.winehq.org/RegressionTesting

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

Petr Dlouhý  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #6 from Petr Dlouhý   2008-04-17 18:32:10 ---
I can confirm, that WZebra 4.2.4 is falling on stack overflow in Wine
0.9.{12,20,30,40,50}.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #7 from Petr Dlouhý   2008-05-08 15:21:45 ---
Created an attachment (id=12832)
 --> (http://bugs.winehq.org/attachment.cgi?id=12832)
menu test

I made conformance test for this, but I am new to Wine, so I am not sure if
everything is OK, and if so, how can I post the test to Wine.
It is testing also two other things, that I thought that could be interesting
(it is described in comments).

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #8 from Petr Dlouhý   2008-05-11 14:03:32 ---
Created an attachment (id=12936)
 --> (http://bugs.winehq.org/attachment.cgi?id=12936)
maximum depth check

I made a patch which tests maximum depth of recursion calling of MENU_FindItem
allowing WZebra to start.
I am new to Wine, so I would be glad if somebody more experienced could take
look on the patch.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

Petr Dlouhý  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #12936|0                           |1
        is obsolete|                            |

--- Comment #9 from Petr Dlouhý   2008-05-11 14:08:20 ---
Created an attachment (id=12937)
 --> (http://bugs.winehq.org/attachment.cgi?id=12937)
maximum depth check

Sorry about the change of encoding in name. I am attaching new patch.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #10 from Petr Dlouhý   2008-05-11 14:17:04 ---
Hm, now I realized, that WZebra starts, but there is “dummy” menu item in
“Zebra Settings” instead of “Time”, so I should make a better patch.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

--- Comment #11 from Austin English   2008-05-11 17:36:46 ---
Please send patches to [e-mail]...They won't get picked up here.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #12 from Dmitry Timoshkov   2008-05-11 21:46:45 ---
Please perform the requested regression test first instead of creating
patches aiming to hunt down a probably non-existent bug.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #13 from Austin English   2008-05-12 00:06:26 ---
Do you know a working version?

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #14 from srepmub   2008-05-12 10:59:58 ---
the last working version (also without a graphics glitch that remains when
hand-hacking around the current problem) I tried was probably well before
0.9.0, possibly even 3 or more years ago.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #15 from Petr Dlouhý   2008-05-12 12:11:51 ---
srepmub: Are sure, it was Wzebra version 4.2.4 and not 4.2.1 which hasn't got
the problem?

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #16 from srepmub   2008-05-12 12:15:59 ---
yeah, it could easily have been an earlier version I guess..

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #17 from Dmitry Timoshkov   2008-05-12 21:01:55 ---
(In reply to comment #14)
> the last working version (also without a graphics glitch that remains when
> hand-hacking around the current problem) I tried was probably well before
> 0.9.0, possibly even 3 or more years ago.

It should still be possible to perform a regression test.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #18 from Petr Dlouhý   2008-05-13 05:10:41 ---
> It should still be possible to perform a regression test.

This problem is only with WZebra version 4.2.4, and version 4.2.1 isn't
affected by this. Srepmub admit, that he don't know with which version of
WZebra it worked:
> yeah, it could easily have been an earlier version I guess..

Wzebra 4.2.4 was released on December 27, 2005
(http://radagast.se/othello/download.html), it is the time where Wine 0.9.4 was
just out
(http://source.winehq.org/git/wine.git/?a=commit;h=4d05f2444a51c2eb4bf75afd9d...).
So if it was tried in version before 0.9.0 it probably can't be WZebra 4.2.4.

I doubt, that this should ever work, because Wine is supposed to find menu item
in cycled structure, and Wine should somehow count with this (limited depth or
additional structure which holds menu items).

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #19 from Petr Dlouhý   2008-06-15 09:31:22 ---
I meant:
that this should ever work → that this could ever work

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #20 from srepmub   2008-07-15 05:41:10 ---
the board colors are now correct with 1.1.1.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

Louis Lenders  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1

--- Comment #21 from Louis Lenders   2008-07-31 05:25:44 ---
this is a reression between wine-0.9.6 (working fine) and wine-0.9.13 ( bad). I
tested wzebra. mark, a regression test should now be possible i guess

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #22 from Dmitry Timoshkov   2008-07-31 07:22:33 ---
To simplify the regression test (and speed it up a lot) limit it by
dlls/user directory, or even by the only dlls/user/menu.c file,

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

Vitaliy Margolen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #23 from Vitaliy Margolen   2009-01-12 11:45:29 ---
This is not a regression. I can't find a working version. But found another
broken app...

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #24 from Rein Klazes   2009-07-16 05:45:06 ---
I have tried the WZebra 2.4.2 application and I find that the problem is not in
the finditem. The real difference with Windows is that the ModifyMenu function
fails when attempting to create a loop in the menu tree.
Since all examples shown are ignoring the return value of ModifyMenu() this is
not noticed.

I have submitted a test that show the behavior of WZebra. Another test shows
that on windows there is a limit of about 25 on the depth of the menu tree,
probably used in the tests for loops in the menu tree.

http://www.winehq.org/pipermail/wine-patches/2009-July/076008.html

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #25 from srepmub   2009-07-16 07:13:49 ---
great, thanks!

I guess with a test failing consistently, we can expect this to be fixed soon..
:)

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #26 from Rein Klazes   2009-07-16 08:32:04 ---
(In reply to comment #25)
> great, thanks!
>
> I guess with a test failing consistently, we can expect this to be fixed soon..
> :)

Yes and No.

Yes: I can fix the stack overflow easy enough. The only thing that is delaying
it, is that I decided to reorganize the menu code a bit first, to simplify the
fix later.

No: the real cause of the problem is that the menu handles in wine clash with
the id's used in the program. Say in the description of this bug the number
1234 happens to the handle of some menu. That confusion will stay but how bad
that is remains to be seen. It cannot be fixed until wine's handles are more
like those in windows.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #27 from Rein Klazes   2009-07-24 02:22:25 ---
Patch against current git submitted:

http://www.winehq.org/pipermail/wine-patches/2009-July/076374.html

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #28 from Austin English   2009-07-24 10:31:55 ---
(In reply to comment #27)
> Patch against current git submitted:
>
> http://www.winehq.org/pipermail/wine-patches/2009-July/076374.html

Patch committed:
http://source.winehq.org/git/wine.git/?a=commitdiff;h=38dbe067c370610d0a2423...

Please retest/close.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

--- Comment #29 from Rein Klazes   2009-07-24 10:43:31 ---
(In reply to comment #28)
> (In reply to comment #27)
> Please retest/close.

Hmmm. The real bug has not been fixed, and it still shows.

That is that in wine menu handles are still 16 bits and their value will clash
with applications where windows will not. 
It shows by in one (at least) place where a submenu is labeled "dummy". The
impact is now reduced to "cosmetic", but it is still a bug.

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

Rein Klazes  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #30 from Rein Klazes   2009-10-14 13:01:17 ---
Fixed by commit: db79a9f2e9a5f4d82c0a910fd92ce8449cdf74e3

[Bug 12171] adding popupmenu to itself, then using finditem causes stack overflow

http://bugs.winehq.org/show_bug.cgi?id=12171

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #31 from Alexandre Julliard   2009-10-23 13:18:23 ---
Closing bugs fixed in 1.1.32.