United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6665028 native code of method j*.text.Bidi.nativeBidiChars is using the contents of a primitive array direct
JDK-6665028 : native code of method j*.text.Bidi.nativeBidiChars is using the contents of a primitive array direct

Details
Type:
Bug
Submit Date:
2008-02-19
Status:
Resolved
Updated Date:
2010-07-29
Project Name:
JDK
Resolved Date:
2008-03-25
Component:
core-libs
OS:
solaris_7
Sub-Component:
java.text
CPU:
sparc
Priority:
P3
Resolution:
Fixed
Affected Versions:
6u4
Fixed Versions:
6u10 (b20)

Related Reports
Backport:
Backport:
Backport:
Backport:
Backport:
Relates:

Sub Tasks

Description
It seems that the native code of method java.text.Bidi.nativeBidiChars is using the contents of a primitive array directly after calling to the JNI function ReleasePrimitiveArrayCritical.

Here are some code analysis in detail.

The definition of native method java.text.Bidi.nativeBidiChars,
Java_java_text_Bidi_nativeBidiChars, is in file
j2se/src/share/native/sun/font/bidi/jbidi.c.
For the case that parameter "embs" is not NULL, cEmbs will be set to the
pointer(got by GetPrimitiveArrayCritical) to the contents of byte array
embs.
In function ubidi_setPara, which is defined in file
j2se/src/share/native/sun/font/bidi/ubidi.c, field "levels" of pBiDi
will be set to embeddingLevels, which
is the value of cEmbs. After calling to function ubidi_setPara,
ReleasePrimitiveArrayCritical is called for the byte array embs, however
the value of
cEmbs is still being held by bidi->levels, and will be used later in
function ubidi_getLogicalRun, defined in file
j2se/src/share/native/sun/font/bidi/ubidiln.c.
Since GC is no longer locked after calling to
ReleasePrimitiveArrayCritical, we think it is possible that the contents
of byte array embs is moved to some
place else, and bidi->levels is pointing to the wrong place.

We suggest that the piece of code in function ubidi_setPara in file
j2se/src/share/native/sun/font/bidi/ubidi.c

------------------------------------------------------------------------
-
    /* are explicit levels specified? */
    if(embeddingLevels==NULL) {
        /* no: determine explicit levels according to the (Xn) rules */\
        if(getLevelsMemory(pBiDi, length)) {
            pBiDi->levels=pBiDi->levelsMemory;
            direction=resolveExplicitLevels(pBiDi);
        } else {
            *pErrorCode=U_MEMORY_ALLOCATION_ERROR;
            return;
        }
    } else {
        /* set BN for all explicit codes, check that all levels are
paraLevel..UBIDI_MAX_EXPLICIT_LEVEL */
        pBiDi->levels=embeddingLevels;
        direction=checkExplicitLevels(pBiDi, pErrorCode);
        if(U_FAILURE(*pErrorCode)) {
            return;
        }
    }
------------------------------------------------------------------------
-

might be changed to

------------------------------------------------------------------------
-
    if(getLevelsMemory(pBiDi, length)) {
        pBiDi->levels=pBiDi->levelsMemory;
        /* are explicit levels specified? */
        if(embeddingLevels==NULL) {
            /* no: determine explicit levels according to the (Xn) rules
\ */
            direction=resolveExplicitLevels(pBiDi);
        } else {
            /* set BN for all explicit codes, check that all levels are
paraLevel..UBIDI_MAX_EXPLICIT_LEVEL */
            icu_memcpy(pBiDi->levels, embeddingLevels, length);
            direction=checkExplicitLevels(pBiDi, pErrorCode);
            if(U_FAILURE(*pErrorCode)) {
                return;
            }
        }
    } else {
        *pErrorCode=U_MEMORY_ALLOCATION_ERROR;
        return;
    }
------------------------------------------------------------------------
-

                                    

Comments
SUGGESTED FIX

We suggest that the piece of code in function ubidi_setPara in file
j2se/src/share/native/sun/font/bidi/ubidi.c

------------------------------------------------------------------------
-
    /* are explicit levels specified? */
    if(embeddingLevels==NULL) {
        /* no: determine explicit levels according to the (Xn) rules */\
        if(getLevelsMemory(pBiDi, length)) {
            pBiDi->levels=pBiDi->levelsMemory;
            direction=resolveExplicitLevels(pBiDi);
        } else {
            *pErrorCode=U_MEMORY_ALLOCATION_ERROR;
            return;
        }
    } else {
        /* set BN for all explicit codes, check that all levels are
paraLevel..UBIDI_MAX_EXPLICIT_LEVEL */
        pBiDi->levels=embeddingLevels;
        direction=checkExplicitLevels(pBiDi, pErrorCode);
        if(U_FAILURE(*pErrorCode)) {
            return;
        }
    }
------------------------------------------------------------------------
-

might be changed to

------------------------------------------------------------------------
-
    if(getLevelsMemory(pBiDi, length)) {
        pBiDi->levels=pBiDi->levelsMemory;
        /* are explicit levels specified? */
        if(embeddingLevels==NULL) {
            /* no: determine explicit levels according to the (Xn) rules
\ */
            direction=resolveExplicitLevels(pBiDi);
        } else {
            /* set BN for all explicit codes, check that all levels are
paraLevel..UBIDI_MAX_EXPLICIT_LEVEL */
            icu_memcpy(pBiDi->levels, embeddingLevels, length);
            direction=checkExplicitLevels(pBiDi, pErrorCode);
            if(U_FAILURE(*pErrorCode)) {
                return;
            }
        }
    } else {
        *pErrorCode=U_MEMORY_ALLOCATION_ERROR;
        return;
    }
------------------------------------------------------------------------
-
*** (#1 of 1): [ UNSAVED ] ###@###.###
                                     
2008-02-19
EVALUATION

Yes, we should take the suggested fix.
                                     
2008-02-20



Hardware and Software, Engineered to Work Together