On Mon, 14 Aug 2023 18:03:21 GMT, Harshitha Onkar <[email protected]> wrote:

> In awt_MenuItem.cpp (712,22): ` mii.dwTypeData = (LPTSTR)(*sb)`  produces 
> invalid pointer cast warning when complied on clang and moreover this is a 
> no-op.  
> 
> `mii.dwTypeData` is used only when **MIIM_STRING** flag is set in the fMask 
> (as per 
> [Docs](https://learn.microsoft.com/en-us/windows/win32/api/winuser/ns-winuser-menuiteminfoa)),
>  which is not the case in JDK 
> [Ln#705](https://github.com/openjdk/jdk/blob/e56d3bc2dab3d32453b6eda66e8434953c436084/src/java.desktop/windows/native/libawt/windows/awt_MenuItem.cpp#L706).
>  Hence the assignment ` mii.dwTypeData = (LPTSTR)(*sb)`  is not required and 
> so is the label parameter. Additionally necessary cleanup is done at the 
> following places -
> 
> - WMenuItemPeer.java - to the native function call
> - awt_MenuItem.cpp -  `WMenuItemPeer__1setLabel() ,_SetLabel(), SetLabel()`
> - awt_MenuItem.h
> 
> Added a test which checks setLabel() functionality on Menu, MenuItem and 
> PopupMenu.

I would also add that the pointer saved to `mii.dwTypeData` becomes invalid as 
soon as `m->SetLabel(labelPtr)` returns because the code in `_SetLabel` 
releases the pointer `labelPtr`.

Essentially, this was the code flow in `_SetLabel`:

LPCTSTR labelPtr = JNU_GetStringPlatformChars(env, label, 0);
m->SetLabel(labelPtr);
JNU_ReleaseStringPlatformChars(env, label, labelPtr);


If any code had dereferenced the pointer stored for a menu item in 
`dwTypeData`, the process would've crashed with access violation, or it 
could've led to a memory corruption.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15276#issuecomment-1680931832

Reply via email to