Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correcting invocation of ScrollConsoleScreenBufferW - using pointers instead of values. #1038

Merged

Conversation

lahodaj
Copy link
Contributor

@lahodaj lahodaj commented Jul 16, 2024

As described in OpenJDK bug:
https://bugs.openjdk.org/browse/JDK-8336375

the invocation of ScrollConsoleScreenBufferW in the FFM implementation of Kernel32 may crash. I believe the reason is that the lookup/"declared" parameters are pointers:

static final MethodHandle ScrollConsoleScreenBufferW$MH = downcallHandle(

but the provided/actual invocation parameters are values:
mh$.invokeExact(hConsoleOutput, lpScrollRectangle, lpClipRectangle, dwDestinationOrigin, lpFill);

In OpenJDK, I proposed to change the code to use .seg, which is consistent with other similar invocations in Kernel32, and should ensure a pointer is sent to the native function:
openjdk/jdk#20182

The patch herein is basically the same as the OpenJDK patch.

(I am not aware of a way to automatically test this.)

@lahodaj lahodaj marked this pull request as draft July 16, 2024 06:38
@lahodaj lahodaj force-pushed the fix-ScrollConsoleScreenBufferW-invocation branch from 9961b81 to 06d5815 Compare July 16, 2024 06:42
@lahodaj lahodaj marked this pull request as ready for review July 16, 2024 06:54
@gnodet gnodet added this to the 3.26.3 milestone Jul 17, 2024
@gnodet
Copy link
Member

gnodet commented Jul 17, 2024

Nice catch, thx @lahodaj

@gnodet gnodet closed this Jul 17, 2024
@gnodet gnodet reopened this Jul 17, 2024
@gnodet gnodet merged commit dad7818 into jline:master Jul 17, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants