Bug 539796 - [Win32] Crash in ScriptStringOut
This fixes the problem.
Signature for ScriptStringAnalyse changed to keep the native string.
Previously, JNI wrapper for ScriptStringAnalyse deallocated temporary
native string upon exit, causing ScriptStringOut to read already-freed memory.
Depending on circumstances that will
1) Read memory that is still intact, making impression that it works fine
2) Read memory overwritten by new owner
3) Crash if entire virtual page was deallocated by Windows
In the original fix for Bug 239477, it was incorrectly assumed that
Uniscribe libraries crash. The true problem is that when a buffer is
big enough (such as 16665 used in test snippet), Windows will most
likely deallocate virtual pages along with the heap block, and subsequent
read from such page will guarantee a crash. With buffer of just 2 characters
the problem is still there, but most often the memory will be intact for
short while after deallocation, because virtual page containing deallocated
block is still occupied with other heap blocks.
Workaround from Bug 239477, that is estimating font by just 2 characters,
has its own side effects. It will be addressed in future patches.
Easily reproducible with Application Verifier configured for Basics/Heaps,
because Application Verifier reduces the chances to access freed memory
to almost zero.
Code snippet that reproduces the problem:
final Display display = new Display();
TextLayout layout = new TextLayout(display);
layout.setText("\u0001");
layout.getBounds();
Change-Id: Ibc5e15b173beca54b2ed73cdcb1bc9eb40d4187d
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/win32/library/os.c b/bundles/org.eclipse.swt/Eclipse SWT PI/win32/library/os.c
index 155ea92..62d201b 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/win32/library/os.c
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/win32/library/os.c
@@ -11862,21 +11862,18 @@
#ifndef NO_ScriptStringAnalyse
JNIEXPORT jint JNICALL OS_NATIVE(ScriptStringAnalyse)
- (JNIEnv *env, jclass that, jintLong arg0, jcharArray arg1, jint arg2, jint arg3, jint arg4, jint arg5, jint arg6, jobject arg7, jobject arg8, jintLong arg9, jintLong arg10, jintLong arg11, jintLong arg12)
+ (JNIEnv *env, jclass that, jintLong arg0, jintLong arg1, jint arg2, jint arg3, jint arg4, jint arg5, jint arg6, jobject arg7, jobject arg8, jintLong arg9, jintLong arg10, jintLong arg11, jintLong arg12)
{
- jchar *lparg1=NULL;
SCRIPT_CONTROL _arg7, *lparg7=NULL;
SCRIPT_STATE _arg8, *lparg8=NULL;
jint rc = 0;
OS_NATIVE_ENTER(env, that, ScriptStringAnalyse_FUNC);
- if (arg1) if ((lparg1 = (*env)->GetCharArrayElements(env, arg1, NULL)) == NULL) goto fail;
if (arg7) if ((lparg7 = getSCRIPT_CONTROLFields(env, arg7, &_arg7)) == NULL) goto fail;
if (arg8) if ((lparg8 = getSCRIPT_STATEFields(env, arg8, &_arg8)) == NULL) goto fail;
- rc = (jint)ScriptStringAnalyse((HDC)arg0, (const void*)lparg1, arg2, arg3, arg4, arg5, arg6, lparg7, lparg8, (const int*)arg9, (SCRIPT_TABDEF*)arg10, (const BYTE*)arg11, (SCRIPT_STRING_ANALYSIS*)arg12);
+ rc = (jint)ScriptStringAnalyse((HDC)arg0, (const void*)arg1, arg2, arg3, arg4, arg5, arg6, lparg7, lparg8, (const int*)arg9, (SCRIPT_TABDEF*)arg10, (const BYTE*)arg11, (SCRIPT_STRING_ANALYSIS*)arg12);
fail:
if (arg8 && lparg8) setSCRIPT_STATEFields(env, arg8, lparg8);
if (arg7 && lparg7) setSCRIPT_CONTROLFields(env, arg7, lparg7);
- if (arg1 && lparg1) (*env)->ReleaseCharArrayElements(env, arg1, lparg1, 0);
OS_NATIVE_EXIT(env, that, ScriptStringAnalyse_FUNC);
return rc;
}
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java b/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java
index 6280d7e..9aa574d 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java
@@ -5450,7 +5450,7 @@
* @param pbInClass cast=(const BYTE*)
* @param pssa cast=(SCRIPT_STRING_ANALYSIS*)
*/
-public static final native int ScriptStringAnalyse (long /*int*/ hdc, char[] pString, int cString, int cGlyphs, int iCharset, int dwFlags, int iReqWidth, SCRIPT_CONTROL psControl, SCRIPT_STATE psState, long /*int*/ piDx, long /*int*/ pTabdef, long /*int*/ pbInClass, long /*int*/ pssa);
+public static final native int ScriptStringAnalyse (long /*int*/ hdc, long /*int*/ pString, int cString, int cGlyphs, int iCharset, int dwFlags, int iReqWidth, SCRIPT_CONTROL psControl, SCRIPT_STATE psState, long /*int*/ piDx, long /*int*/ pTabdef, long /*int*/ pbInClass, long /*int*/ pssa);
/** @param ssa cast=(SCRIPT_STRING_ANALYSIS*),flags=struct */
public static final native int ScriptStringOut(long /*int*/ ssa, int iX, int iY, int uOptions, RECT prc, int iMinSel, int iMaxSel, boolean fDisabled);
/** @param pssa cast=(SCRIPT_STRING_ANALYSIS*) */
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/TextLayout.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/TextLayout.java
index 74a756a..aae6731 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/TextLayout.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/TextLayout.java
@@ -3406,14 +3406,29 @@
long /*int*/ createMetafileWithChars(long /*int*/ hdc, long /*int*/ hFont, char[] chars, int charCount) {
long /*int*/ hHeap = OS.GetProcessHeap();
+
+ /*
+ * The native string must remain unchanged between ScriptStringAnalyse and ScriptStringOut.
+ * According to debugging, ScriptStringAnalyse implicitly saves string to SCRIPT_STRING_ANALYSIS.
+ * Then, ScriptStringOut uses the saved string in internal call to ExtTextOutW.
+ * I believe this is due to OS.SSA_METAFILE, which is documented as follows:
+ * Write items with ExtTextOutW calls, not with glyphs.
+ * Note: passing Java chars to native function is wrong, because JNI will allocate
+ * temporary native string which will be deallocated upon return from ScriptStringAnalyse.
+ */
+ int nativeStringSize = charCount * Character.BYTES;
+ long /*int*/ nativeString = OS.HeapAlloc (hHeap, OS.HEAP_ZERO_MEMORY, nativeStringSize);
+ OS.MoveMemory (nativeString, chars, nativeStringSize);
+
long /*int*/ ssa = OS.HeapAlloc(hHeap, OS.HEAP_ZERO_MEMORY, OS.SCRIPT_STRING_ANALYSIS_sizeof());
long /*int*/ metaFileDc = OS.CreateEnhMetaFile(hdc, null, null, null);
long /*int*/ oldMetaFont = OS.SelectObject(metaFileDc, hFont);
int flags = OS.SSA_METAFILE | OS.SSA_FALLBACK | OS.SSA_GLYPHS | OS.SSA_LINK;
- if (OS.ScriptStringAnalyse(metaFileDc, chars, charCount, 0, -1, flags, 0, null, null, 0, 0, 0, ssa) == OS.S_OK) {
+ if (OS.ScriptStringAnalyse(metaFileDc, nativeString, charCount, 0, -1, flags, 0, null, null, 0, 0, 0, ssa) == OS.S_OK) {
OS.ScriptStringOut(ssa, 0, 0, 0, null, 0, 0, false);
OS.ScriptStringFree(ssa);
}
+ OS.HeapFree(hHeap, 0, nativeString);
OS.HeapFree(hHeap, 0, ssa);
OS.SelectObject(metaFileDc, oldMetaFont);
return OS.CloseEnhMetaFile(metaFileDc);
