[drlvm][jni] Summary of Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
So to summarize so we can close this. I assume that we all agree that the JNI spec is lacking, and that we should follow the RI as it's reasonable behavior. Gregory has added a patch to HARMONY-1156 and I'm going to accept and apply that. If there is any opposition, please speak now. geir Ivan Volosyuk wrote: > +1 for the following check: > >> a) offset is negative or >> b) len is negative or >> c) off + len > buff.length > > -- > Ivan > > On 8/14/06, Geir Magnusson Jr <[EMAIL PROTECTED]> wrote: >> Ivan Volosyuk wrote: >> > It looks like that it is possible to get zero bytes even at the very >> > end of array. IMHO the order of boundary checks makes sense here. >> >> This is an interesting problem. The JNI spec is clear that we should >> throw an exception when one of the indexes isn't valid, and start == >> size is arguably not valid, as Gregory points out. >> >> So I think the JNI impl is right for now (but I want to test w/ the JNI >> impl in RI) >> >> As for classlibrary : >> >> I was testing with the RI vi java.io.OutputStream#write(buff[], offset, >> len), and the RI is clearly broken as it doesn't throw an NPE when [] is >> null. >> >> However, strictly speaking, an IndexOOBE is thrown when : >> >> a) offset is negative or >> b) len is negative or >> c) off + len > buff.length >> >> So in the case of >> >>write( byte[count], count, 0); >> >> for any value of count >= 0, then none of the conditions are true, and >> it should simply [quickly] return. >> >> Therefore, I'll start a new thread re refining OutputStream to match >> those rules, and then go on to test the JNI impl of the RI. >> >> geir >> >> > -- >> > Ivan >> > >> > On 8/11/06, Gregory Shimansky <[EMAIL PROTECTED]> wrote: >> >> 2006/8/11, Jimmy, Jing Lv <[EMAIL PROTECTED]>: >> >> > >> >> > Hi, >> >> > >> >> > As discussed in the former thread, I find that a JNI of DRLVM >> >> > (GetByteArrayRegion) differs from RI in passing parameter >> >> > (byte[count],count,0). RI (and J9 VM) returns immediately but DRLVM >> >> > throws an ArrayIndexOutOfBoundsException. >> >> > IMHO, it is better to improve here, make it follow RI, as >> JNI is >> >> > also a part of API, and users may use it. So if no objections, >> shall I >> >> > or someone raise a JIRA for this? >> >> > >> >> > I don't know if Martin is urgent in using Harmony in Winstone >> >> > servlet engine (Martin, can you read this?), so shall I work >> around in >> >> > java code before we have conclusion? >> >> > >> >> > Any comments/suggestions from DRLVM guys? Thanks! >> >> > >> >> >> >> I didn't really understand the condition when vm throws AIOBE. Do you >> >> pass >> >> start parameter count for the array of count length? But in this >> case the >> >> start points to the element right after the array end. And spec [1] >> >> explicitly states that this function should throw AIOBE in this >> case. Is >> >> there something I don't understand? >> >> >> >> [1] >> >> >> http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/functions.html#wp6212 > > - > Terms of use : http://incubator.apache.org/harmony/mailing.html > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > > - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
+1 for the following check: a) offset is negative or b) len is negative or c) off + len > buff.length -- Ivan On 8/14/06, Geir Magnusson Jr <[EMAIL PROTECTED]> wrote: Ivan Volosyuk wrote: > It looks like that it is possible to get zero bytes even at the very > end of array. IMHO the order of boundary checks makes sense here. This is an interesting problem. The JNI spec is clear that we should throw an exception when one of the indexes isn't valid, and start == size is arguably not valid, as Gregory points out. So I think the JNI impl is right for now (but I want to test w/ the JNI impl in RI) As for classlibrary : I was testing with the RI vi java.io.OutputStream#write(buff[], offset, len), and the RI is clearly broken as it doesn't throw an NPE when [] is null. However, strictly speaking, an IndexOOBE is thrown when : a) offset is negative or b) len is negative or c) off + len > buff.length So in the case of write( byte[count], count, 0); for any value of count >= 0, then none of the conditions are true, and it should simply [quickly] return. Therefore, I'll start a new thread re refining OutputStream to match those rules, and then go on to test the JNI impl of the RI. geir > -- > Ivan > > On 8/11/06, Gregory Shimansky <[EMAIL PROTECTED]> wrote: >> 2006/8/11, Jimmy, Jing Lv <[EMAIL PROTECTED]>: >> > >> > Hi, >> > >> > As discussed in the former thread, I find that a JNI of DRLVM >> > (GetByteArrayRegion) differs from RI in passing parameter >> > (byte[count],count,0). RI (and J9 VM) returns immediately but DRLVM >> > throws an ArrayIndexOutOfBoundsException. >> > IMHO, it is better to improve here, make it follow RI, as JNI is >> > also a part of API, and users may use it. So if no objections, shall I >> > or someone raise a JIRA for this? >> > >> > I don't know if Martin is urgent in using Harmony in Winstone >> > servlet engine (Martin, can you read this?), so shall I work around in >> > java code before we have conclusion? >> > >> > Any comments/suggestions from DRLVM guys? Thanks! >> > >> >> I didn't really understand the condition when vm throws AIOBE. Do you >> pass >> start parameter count for the array of count length? But in this case the >> start points to the element right after the array end. And spec [1] >> explicitly states that this function should throw AIOBE in this case. Is >> there something I don't understand? >> >> [1] >> http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/functions.html#wp6212 - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
2006/8/15, Jimmy, Jing Lv <[EMAIL PROTECTED]>: IMHO our Compatibility guidelines, if the spec is not clear, we should follow RI. So no matter what happens to the spec(unless it describe the detail condition of exception-thrown), it is still OK to follow RI here, am I right? Ok you didn't convince me which spec if more correct but to close this discussion I agree to create a patch to JIRA 1156 to allow start index equal to array length in case the len argument is 0. In classlib discussions I've seen we agreed to document places where we chose to follow RI instead of public spec. Is there any place where we will document these conditions for VMs? The whole exception condition looks like this > > jsize length = GetArrayLength(env, array); > jsize end = start + len; > if(start < 0 || start >= length || end < 0 || end > length) { > char msg[30]; > sprintf(msg, "%d..%d", start, end); > ThrowNew_Quick(env, "java/lang/ArrayIndexOutOfBoundsException", msg); > return; > } > > and it is easy to change start >= length to start > length if you ask for it. IMHO there's something wrong here. What will it do if len<0 and length >start+len >0 ? Shall it be: if(start < 0 || start > length || len < 0 || end > length)? And the next line shall be like: if (0 == len){ return; } No. There is assert(len >= 0) right before the code snippet which I pasted. JNI does not allow users to pass incorrect arguments like null pointers and bad indexes so it is legal to use assertions in its implementation [1] (another link to JNI spec which doesn't convince anyone here any more). So the code correction which I proposed seems to be the only needed one. I don't think it is necessary to handle len == 0 specifically to avoid additional checks for a rare (hopefully) case. I am still unsure if this is a place where spec should step away from the > spec be it imcomplete or not. Programmers who don't work for Sun read public > spec, don't they? > > [1] http://java.sun.com/docs/books/jni/html/jniTOC.html > Still we should follow our Compatibility guidelines, right(For our goal of Harmony)? :) These guidelines were so far dicussed for classlib only. While I agree with most of them they mention only classlib implementation so far. [1] http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/design.html#wp17593 -- Gregory Shimansky, Intel Middleware Products Division
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
Gregory Shimansky wrote: On Monday 14 August 2006 23:37 Geir Magnusson Jr wrote: I've written a test [1] myself and cannot say I completely understand the result. With length = 0 RI 1.5 allows calling to GetArrayRegion with start equal to array length but throws AIOOBE if start is greater than array length. That makes sense to me, only because I am thinking of j.i.OutputStream's write([], int, int) method, which does state that it's ok if start + len == arraylen... It is not specified either, is it? I looked at the ObjectOutputStream.write(byte[], int, int) and didn't find any detailed description about allowed ranges. That's why the compatibility guidelines are developed on same issue for classlib[1]. I'm sure if we thought about it, we'd figure out that it lends itself nicely to some common loop idiom. I suspect it will be some end case when some read returns an empty buffer, so write(buf, 0, 0) works without a check, or some situation where there's some post decrement leading you to write(buf, length, len) where the len was calculated from (buf.length - length) or something. Now, that isn't what the JNI spec says, but it seems like the JNI spec was written in a hurry... :) JNI spec is indeed quite incomplete when it comes to border cases. Sun wouldn't need to create a special book [1] (however this clarification doesn't clarify this particular case) for JNI clarification if they wrote a complete spec from the start. Although I agree with you that Sun didn't show enough patience on spec writing, I don't believe any thorough specification can be written in native human language, if someone did, others probably refused to read only because of volume...so the way it is... However Sun usually changes a spec if its implementation doesn't work according to it for some time. JNI spec is really old, it was written for 1.1 and the statement about exception still remains. that's true, and that's why we need to pay more attention on RI's behavior... I am unsure if we want to allow this compatibility and a reason to allow it. When length is 0 the application still gets nothing except for clear exception status. There is no value in allowing this call except for allowing software which has a bug in it to work. On the other hand allowing start == length to pass violates the spec IMHO. I think it is better if software which uses this undocumented feature was fixed instead of introducing this workaround, so if others agree I think HARMONY-1156 could be closed. I'm not sure the border case should be considered same as undocumented feature. sun.* classes are undocumented features, but write(null, 0, -1) is just exceptional case. They are different because there is chance that we don't support "sun.*" classes without impairing our reputation, while we cannot refuse to handle the border case if we care about quality. Well, I don't feel strongly either way, but am uncomfortable with the inconsistency. The JNI docs seem pretty sparse, and clearly some thought went into allowing : write( buff, buff.length, 0) The whole exception condition looks like this jsize length = GetArrayLength(env, array); jsize end = start + len; if(start < 0 || start >= length || end < 0 || end > length) { char msg[30]; sprintf(msg, "%d..%d", start, end); ThrowNew_Quick(env, "java/lang/ArrayIndexOutOfBoundsException", msg); return; } and it is easy to change start >= length to start > length if you ask for it. I am still unsure if this is a place where spec should step away from the spec be it imcomplete or not. Programmers who don't work for Sun read public spec, don't they? Yes, they read spec, and they also write small test case to explore what happened for unspecified exceptional case, what exception RI throw when write(null, 0, 1)? NPE or IAE? if we don't write small test like them, we are doomed. [1] http://java.sun.com/docs/books/jni/html/jniTOC.html [1] http://incubator.apache.org/harmony/subcomponents/classlibrary/compat.html -- Paulex Yang China Software Development Lab IBM - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
Gregory Shimansky wrote: > On Monday 14 August 2006 23:37 Geir Magnusson Jr wrote: >>> I've written a test [1] myself and cannot say I completely understand the >>> result. With length = 0 RI 1.5 allows calling to GetArrayRegion >>> with start equal to array length but throws AIOOBE if start is greater >>> than array length. >> That makes sense to me, only because I am thinking of j.i.OutputStream's >> write([], int, int) method, which does state that it's ok if start + len >> == arraylen... > > It is not specified either, is it? I looked at the > ObjectOutputStream.write(byte[], int, int) and didn't find any detailed > description about allowed ranges. I looked at java.io.OutputStream write() and it was clear (to me anyway...) > >> I'm sure if we thought about it, we'd figure out that it lends itself >> nicely to some common loop idiom. I suspect it will be some end case >> when some read returns an empty buffer, so >> >> write(buf, 0, 0) >> >> works without a check, or some situation where there's some post >> decrement leading you to >> >> write(buf, length, len) >> >> where the len was calculated from (buf.length - length) or something. >> >> Now, that isn't what the JNI spec says, but it seems like the JNI spec >> was written in a hurry... :) > > JNI spec is indeed quite incomplete when it comes to border cases. Sun > wouldn't need to create a special book [1] (however this clarification > doesn't clarify this particular case) for JNI clarification if they wrote a > complete spec from the start. > > However Sun usually changes a spec if its implementation doesn't work > according to it for some time. JNI spec is really old, it was written for 1.1 > > and the statement about exception still remains. Yah, but like I said, it's a reasonable change, and it's consistent w/ the RI... > >>> I am unsure if we want to allow this compatibility and a reason to allow >>> it. When length is 0 the application still gets nothing except for clear >>> exception status. There is no value in allowing this call except for >>> allowing software which has a bug in it to work. On the other hand >>> allowing start == length to pass violates the spec IMHO. >>> >>> I think it is better if software which uses this undocumented feature was >>> fixed instead of introducing this workaround, so if others agree I think >>> HARMONY-1156 could be closed. >> Well, I don't feel strongly either way, but am uncomfortable with the >> inconsistency. The JNI docs seem pretty sparse, and clearly some >> thought went into allowing : >> >> write( buff, buff.length, 0) > > The whole exception condition looks like this > > jsize length = GetArrayLength(env, array); > jsize end = start + len; > if(start < 0 || start >= length || end < 0 || end > length) { > char msg[30]; > sprintf(msg, "%d..%d", start, end); > ThrowNew_Quick(env, "java/lang/ArrayIndexOutOfBoundsException", msg); > return; > } > > and it is easy to change start >= length to start > length if you ask for it. but that's not right - a) if you believe that we should be like OutputStream, if start == length and len == 0 you should be ok b) if not, then start == length is wrong as it's not a valid value > I am still unsure if this is a place where spec should step away from the > spec be it imcomplete or not. Programmers who don't work for Sun read public > spec, don't they? > > [1] http://java.sun.com/docs/books/jni/html/jniTOC.html > Who uses JNI? :) geir - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
Please correct me if I'm wrong, I thought what you mentioned as "compatibility guideline" is this[1], as the url shows, it's so far only classlib compatibility guideline, though I guess most of them makes sense for VM, but we may need some discussion to decide, let the VM developers speak for themselves on this issue. [1] http://incubator.apache.org/harmony/subcomponents/classlibrary/compat.html Jimmy, Jing Lv wrote: Gregory Shimansky wrote: On Monday 14 August 2006 23:37 Geir Magnusson Jr wrote: I've written a test [1] myself and cannot say I completely understand the result. With length = 0 RI 1.5 allows calling to GetArrayRegion with start equal to array length but throws AIOOBE if start is greater than array length. That makes sense to me, only because I am thinking of j.i.OutputStream's write([], int, int) method, which does state that it's ok if start + len == arraylen... It is not specified either, is it? I looked at the ObjectOutputStream.write(byte[], int, int) and didn't find any detailed description about allowed ranges. I'm sure if we thought about it, we'd figure out that it lends itself nicely to some common loop idiom. I suspect it will be some end case when some read returns an empty buffer, so write(buf, 0, 0) works without a check, or some situation where there's some post decrement leading you to write(buf, length, len) where the len was calculated from (buf.length - length) or something. Now, that isn't what the JNI spec says, but it seems like the JNI spec was written in a hurry... :) JNI spec is indeed quite incomplete when it comes to border cases. Sun wouldn't need to create a special book [1] (however this clarification doesn't clarify this particular case) for JNI clarification if they wrote a complete spec from the start. However Sun usually changes a spec if its implementation doesn't work according to it for some time. JNI spec is really old, it was written for 1.1 and the statement about exception still remains. Hi, IMHO our Compatibility guidelines, if the spec is not clear, we should follow RI. So no matter what happens to the spec(unless it describe the detail condition of exception-thrown), it is still OK to follow RI here, am I right? I am unsure if we want to allow this compatibility and a reason to allow it. When length is 0 the application still gets nothing except for clear exception status. There is no value in allowing this call except for allowing software which has a bug in it to work. On the other hand allowing start == length to pass violates the spec IMHO. I think it is better if software which uses this undocumented feature was fixed instead of introducing this workaround, so if others agree I think HARMONY-1156 could be closed. Well, I don't feel strongly either way, but am uncomfortable with the inconsistency. The JNI docs seem pretty sparse, and clearly some thought went into allowing : write( buff, buff.length, 0) The whole exception condition looks like this jsize length = GetArrayLength(env, array); jsize end = start + len; if(start < 0 || start >= length || end < 0 || end > length) { char msg[30]; sprintf(msg, "%d..%d", start, end); ThrowNew_Quick(env, "java/lang/ArrayIndexOutOfBoundsException", msg); return; } and it is easy to change start >= length to start > length if you ask for it. IMHO there's something wrong here. What will it do if len<0 and length >start+len >0 ? Shall it be: if(start < 0 || start > length || len < 0 || end > length)? And the next line shall be like: if (0 == len){ return; } I am still unsure if this is a place where spec should step away from the spec be it imcomplete or not. Programmers who don't work for Sun read public spec, don't they? [1] http://java.sun.com/docs/books/jni/html/jniTOC.html Still we should follow our Compatibility guidelines, right(For our goal of Harmony)? :) -- Paulex Yang China Software Development Lab IBM - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
Gregory Shimansky wrote: On Monday 14 August 2006 23:37 Geir Magnusson Jr wrote: I've written a test [1] myself and cannot say I completely understand the result. With length = 0 RI 1.5 allows calling to GetArrayRegion with start equal to array length but throws AIOOBE if start is greater than array length. That makes sense to me, only because I am thinking of j.i.OutputStream's write([], int, int) method, which does state that it's ok if start + len == arraylen... It is not specified either, is it? I looked at the ObjectOutputStream.write(byte[], int, int) and didn't find any detailed description about allowed ranges. I'm sure if we thought about it, we'd figure out that it lends itself nicely to some common loop idiom. I suspect it will be some end case when some read returns an empty buffer, so write(buf, 0, 0) works without a check, or some situation where there's some post decrement leading you to write(buf, length, len) where the len was calculated from (buf.length - length) or something. Now, that isn't what the JNI spec says, but it seems like the JNI spec was written in a hurry... :) JNI spec is indeed quite incomplete when it comes to border cases. Sun wouldn't need to create a special book [1] (however this clarification doesn't clarify this particular case) for JNI clarification if they wrote a complete spec from the start. However Sun usually changes a spec if its implementation doesn't work according to it for some time. JNI spec is really old, it was written for 1.1 and the statement about exception still remains. Hi, IMHO our Compatibility guidelines, if the spec is not clear, we should follow RI. So no matter what happens to the spec(unless it describe the detail condition of exception-thrown), it is still OK to follow RI here, am I right? I am unsure if we want to allow this compatibility and a reason to allow it. When length is 0 the application still gets nothing except for clear exception status. There is no value in allowing this call except for allowing software which has a bug in it to work. On the other hand allowing start == length to pass violates the spec IMHO. I think it is better if software which uses this undocumented feature was fixed instead of introducing this workaround, so if others agree I think HARMONY-1156 could be closed. Well, I don't feel strongly either way, but am uncomfortable with the inconsistency. The JNI docs seem pretty sparse, and clearly some thought went into allowing : write( buff, buff.length, 0) The whole exception condition looks like this jsize length = GetArrayLength(env, array); jsize end = start + len; if(start < 0 || start >= length || end < 0 || end > length) { char msg[30]; sprintf(msg, "%d..%d", start, end); ThrowNew_Quick(env, "java/lang/ArrayIndexOutOfBoundsException", msg); return; } and it is easy to change start >= length to start > length if you ask for it. IMHO there's something wrong here. What will it do if len<0 and length >start+len >0 ? Shall it be: if(start < 0 || start > length || len < 0 || end > length)? And the next line shall be like: if (0 == len){ return; } I am still unsure if this is a place where spec should step away from the spec be it imcomplete or not. Programmers who don't work for Sun read public spec, don't they? [1] http://java.sun.com/docs/books/jni/html/jniTOC.html Still we should follow our Compatibility guidelines, right(For our goal of Harmony)? :) -- Best Regards! Jimmy, Jing Lv China Software Development Lab, IBM - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
On Monday 14 August 2006 23:37 Geir Magnusson Jr wrote: > > I've written a test [1] myself and cannot say I completely understand the > > result. With length = 0 RI 1.5 allows calling to GetArrayRegion > > with start equal to array length but throws AIOOBE if start is greater > > than array length. > > That makes sense to me, only because I am thinking of j.i.OutputStream's > write([], int, int) method, which does state that it's ok if start + len > == arraylen... It is not specified either, is it? I looked at the ObjectOutputStream.write(byte[], int, int) and didn't find any detailed description about allowed ranges. > I'm sure if we thought about it, we'd figure out that it lends itself > nicely to some common loop idiom. I suspect it will be some end case > when some read returns an empty buffer, so > > write(buf, 0, 0) > > works without a check, or some situation where there's some post > decrement leading you to > > write(buf, length, len) > > where the len was calculated from (buf.length - length) or something. > > Now, that isn't what the JNI spec says, but it seems like the JNI spec > was written in a hurry... :) JNI spec is indeed quite incomplete when it comes to border cases. Sun wouldn't need to create a special book [1] (however this clarification doesn't clarify this particular case) for JNI clarification if they wrote a complete spec from the start. However Sun usually changes a spec if its implementation doesn't work according to it for some time. JNI spec is really old, it was written for 1.1 and the statement about exception still remains. > > I am unsure if we want to allow this compatibility and a reason to allow > > it. When length is 0 the application still gets nothing except for clear > > exception status. There is no value in allowing this call except for > > allowing software which has a bug in it to work. On the other hand > > allowing start == length to pass violates the spec IMHO. > > > > I think it is better if software which uses this undocumented feature was > > fixed instead of introducing this workaround, so if others agree I think > > HARMONY-1156 could be closed. > > Well, I don't feel strongly either way, but am uncomfortable with the > inconsistency. The JNI docs seem pretty sparse, and clearly some > thought went into allowing : > > write( buff, buff.length, 0) The whole exception condition looks like this jsize length = GetArrayLength(env, array); jsize end = start + len; if(start < 0 || start >= length || end < 0 || end > length) { char msg[30]; sprintf(msg, "%d..%d", start, end); ThrowNew_Quick(env, "java/lang/ArrayIndexOutOfBoundsException", msg); return; } and it is easy to change start >= length to start > length if you ask for it. I am still unsure if this is a place where spec should step away from the spec be it imcomplete or not. Programmers who don't work for Sun read public spec, don't they? [1] http://java.sun.com/docs/books/jni/html/jniTOC.html -- Gregory Shimansky, Intel Middleware Products Division - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
Gregory Shimansky wrote: > On Monday 14 August 2006 20:13 Geir Magnusson Jr wrote: >> Ivan Volosyuk wrote: >>> It looks like that it is possible to get zero bytes even at the very >>> end of array. IMHO the order of boundary checks makes sense here. >> This is an interesting problem. The JNI spec is clear that we should >> throw an exception when one of the indexes isn't valid, and start == >> size is arguably not valid, as Gregory points out. >> >> So I think the JNI impl is right for now (but I want to test w/ the JNI >> impl in RI) > > I've written a test [1] myself and cannot say I completely understand the > result. With length = 0 RI 1.5 allows calling to GetArrayRegion with > start equal to array length but throws AIOOBE if start is greater than array > length. That makes sense to me, only because I am thinking of j.i.OutputStream's write([], int, int) method, which does state that it's ok if start + len == arraylen... I'm sure if we thought about it, we'd figure out that it lends itself nicely to some common loop idiom. I suspect it will be some end case when some read returns an empty buffer, so write(buf, 0, 0) works without a check, or some situation where there's some post decrement leading you to write(buf, length, len) where the len was calculated from (buf.length - length) or something. Now, that isn't what the JNI spec says, but it seems like the JNI spec was written in a hurry... :) > > I am unsure if we want to allow this compatibility and a reason to allow it. > When length is 0 the application still gets nothing except for clear > exception status. There is no value in allowing this call except for allowing > software which has a bug in it to work. On the other hand allowing start == > length to pass violates the spec IMHO. > > I think it is better if software which uses this undocumented feature was > fixed instead of introducing this workaround, so if others agree I think > HARMONY-1156 could be closed. Well, I don't feel strongly either way, but am uncomfortable with the inconsistency. The JNI docs seem pretty sparse, and clearly some thought went into allowing : write( buff, buff.length, 0) geir - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
On Monday 14 August 2006 20:13 Geir Magnusson Jr wrote: > Ivan Volosyuk wrote: > > It looks like that it is possible to get zero bytes even at the very > > end of array. IMHO the order of boundary checks makes sense here. > > This is an interesting problem. The JNI spec is clear that we should > throw an exception when one of the indexes isn't valid, and start == > size is arguably not valid, as Gregory points out. > > So I think the JNI impl is right for now (but I want to test w/ the JNI > impl in RI) I've written a test [1] myself and cannot say I completely understand the result. With length = 0 RI 1.5 allows calling to GetArrayRegion with start equal to array length but throws AIOOBE if start is greater than array length. I am unsure if we want to allow this compatibility and a reason to allow it. When length is 0 the application still gets nothing except for clear exception status. There is no value in allowing this call except for allowing software which has a bug in it to work. On the other hand allowing start == length to pass violates the spec IMHO. I think it is better if software which uses this undocumented feature was fixed instead of introducing this workaround, so if others agree I think HARMONY-1156 could be closed. [1] public class AIOOBE { static { System.loadLibrary("AIOOBE"); } private native void nativeMethod(int array[], int start); public static void main(String args[]) { int array[] = new int[10]; System.out.println("Calling with length 10"); (new AIOOBE()).nativeMethod(array, 10); System.out.println("Calling with length 11"); (new AIOOBE()).nativeMethod(array, 11); System.out.println("Calling with length 100"); (new AIOOBE()).nativeMethod(array, 100); } } /* DO NOT EDIT THIS FILE - it is machine generated */ #include /* Header for class AIOOBE */ #ifndef _Included_AIOOBE #define _Included_AIOOBE #ifdef __cplusplus extern "C" { #endif /* * Class: AIOOBE * Method:nativeMethod * Signature: ([II)V */ JNIEXPORT void JNICALL Java_AIOOBE_nativeMethod (JNIEnv *jenv, jobject thisObject, jintArray array, jint start) { int buffer[1024]; (*jenv)->GetIntArrayRegion(jenv, array, start, 0, buffer); } #ifdef __cplusplus } #endif #endif -- Gregory Shimansky, Intel Middleware Products Division - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
Ivan Volosyuk wrote: > It looks like that it is possible to get zero bytes even at the very > end of array. IMHO the order of boundary checks makes sense here. This is an interesting problem. The JNI spec is clear that we should throw an exception when one of the indexes isn't valid, and start == size is arguably not valid, as Gregory points out. So I think the JNI impl is right for now (but I want to test w/ the JNI impl in RI) As for classlibrary : I was testing with the RI vi java.io.OutputStream#write(buff[], offset, len), and the RI is clearly broken as it doesn't throw an NPE when [] is null. However, strictly speaking, an IndexOOBE is thrown when : a) offset is negative or b) len is negative or c) off + len > buff.length So in the case of write( byte[count], count, 0); for any value of count >= 0, then none of the conditions are true, and it should simply [quickly] return. Therefore, I'll start a new thread re refining OutputStream to match those rules, and then go on to test the JNI impl of the RI. geir > -- > Ivan > > On 8/11/06, Gregory Shimansky <[EMAIL PROTECTED]> wrote: >> 2006/8/11, Jimmy, Jing Lv <[EMAIL PROTECTED]>: >> > >> > Hi, >> > >> > As discussed in the former thread, I find that a JNI of DRLVM >> > (GetByteArrayRegion) differs from RI in passing parameter >> > (byte[count],count,0). RI (and J9 VM) returns immediately but DRLVM >> > throws an ArrayIndexOutOfBoundsException. >> > IMHO, it is better to improve here, make it follow RI, as JNI is >> > also a part of API, and users may use it. So if no objections, shall I >> > or someone raise a JIRA for this? >> > >> > I don't know if Martin is urgent in using Harmony in Winstone >> > servlet engine (Martin, can you read this?), so shall I work around in >> > java code before we have conclusion? >> > >> > Any comments/suggestions from DRLVM guys? Thanks! >> > >> >> I didn't really understand the condition when vm throws AIOBE. Do you >> pass >> start parameter count for the array of count length? But in this case the >> start points to the element right after the array end. And spec [1] >> explicitly states that this function should throw AIOBE in this case. Is >> there something I don't understand? >> >> [1] >> http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/functions.html#wp6212 >> >> -- >> Gregory Shimansky, Intel Middleware Products Division >> >> > > - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
It looks like that it is possible to get zero bytes even at the very end of array. IMHO the order of boundary checks makes sense here. -- Ivan On 8/11/06, Gregory Shimansky <[EMAIL PROTECTED]> wrote: 2006/8/11, Jimmy, Jing Lv <[EMAIL PROTECTED]>: > > Hi, > > As discussed in the former thread, I find that a JNI of DRLVM > (GetByteArrayRegion) differs from RI in passing parameter > (byte[count],count,0). RI (and J9 VM) returns immediately but DRLVM > throws an ArrayIndexOutOfBoundsException. > IMHO, it is better to improve here, make it follow RI, as JNI is > also a part of API, and users may use it. So if no objections, shall I > or someone raise a JIRA for this? > > I don't know if Martin is urgent in using Harmony in Winstone > servlet engine (Martin, can you read this?), so shall I work around in > java code before we have conclusion? > > Any comments/suggestions from DRLVM guys? Thanks! > I didn't really understand the condition when vm throws AIOBE. Do you pass start parameter count for the array of count length? But in this case the start points to the element right after the array end. And spec [1] explicitly states that this function should throw AIOBE in this case. Is there something I don't understand? [1] http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/functions.html#wp6212 -- Gregory Shimansky, Intel Middleware Products Division -- Ivan Intel Middleware Products Division - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
2006/8/11, Jimmy, Jing Lv <[EMAIL PROTECTED]>: Hi, As discussed in the former thread, I find that a JNI of DRLVM (GetByteArrayRegion) differs from RI in passing parameter (byte[count],count,0). RI (and J9 VM) returns immediately but DRLVM throws an ArrayIndexOutOfBoundsException. IMHO, it is better to improve here, make it follow RI, as JNI is also a part of API, and users may use it. So if no objections, shall I or someone raise a JIRA for this? I don't know if Martin is urgent in using Harmony in Winstone servlet engine (Martin, can you read this?), so shall I work around in java code before we have conclusion? Any comments/suggestions from DRLVM guys? Thanks! I didn't really understand the condition when vm throws AIOBE. Do you pass start parameter count for the array of count length? But in this case the start points to the element right after the array end. And spec [1] explicitly states that this function should throw AIOBE in this case. Is there something I don't understand? [1] http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/functions.html#wp6212 -- Gregory Shimansky, Intel Middleware Products Division
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
I think we should just fix it in DRLVM, although performance-wise, wouldn't also fixing in Java make sense too? As Tim said, raise a JIRA. geir Jimmy, Jing Lv wrote: > Hi, > > As discussed in the former thread, I find that a JNI of DRLVM > (GetByteArrayRegion) differs from RI in passing parameter > (byte[count],count,0). RI (and J9 VM) returns immediately but DRLVM > throws an ArrayIndexOutOfBoundsException. > IMHO, it is better to improve here, make it follow RI, as JNI is > also a part of API, and users may use it. So if no objections, shall I > or someone raise a JIRA for this? > > I don't know if Martin is urgent in using Harmony in Winstone > servlet engine (Martin, can you read this?), so shall I work around in > java code before we have conclusion? > > Any comments/suggestions from DRLVM guys? Thanks! > > > Jimmy, Jing Lv wrote: >> Jimmy, Jing Lv wrote: >>> >>> >>> I do some further study and test then, and find the problem was not >>> so easy. >>> >>> Alex and I are correct that "offset <= buffer.length" here is wrong, >>> but the next "count <= buffer.length - offset" seems has proved its >>> correctness. >>> >>> I have a test[1], try to write(new byte[count],count, 0) to a >>> SocketOutputStream, the test passes quietly. I do this test on WinXp >>> Sp2, the latest Harmony workspace, with J9 VM5. >>> >>> I believe the ArrayIndexOutOfBoundsException is throw out when it try >>> to get byte array in the native (GetByteArrayRegion), which is a JNI >>> method. I guess there may be some difference between VMs. >>> >>> Martin, are you using DRLVM? Can someone using DRLVM (or other VMs) >>> run the test below on DRLVM for me? Thanks! >>> >>> [1] >>> public void test_socketOutputStream() throws Exception { >>> ServerSocket ss = new ServerSocket(0); >>> Socket sock = new Socket(); >>> sock.connect(new InetSocketAddress >>>(InetAddress.getLocalHost(),ss.getLocalPort())); >>> ss.accept(); >>> OutputStream os = sock.getOutputStream(); >>> os.write(new byte[0], 0, 0); // passes here >>> os.write(new byte[512], 512, 0); // passes here >>> } >>> >> >> Hi, >> >> At last I have test[1] JNI method(GetByteArrayRegion) of RI >> (before dinner, very hungry... :) ) >> The result is that, RI return successfully if the given offset >> equals length of the byte array, and the given count is zero. >> >> Thus IMHO it is better to improve DRLVM to follow RI, as JNI >> methods are also API methods, users may use it. >> >> Shall we raise a JIRA for DRLVM? Any comments/suggestions from >> DRLVM guys? Thanks! >> >> Time for dinner! :D >> >> [1] >> test.java >> -- >> class test >> { >> static{ >> System.loadLibrary("testjni"); >> } >>private native void testjni(byte[] bs); >> public void usejni(){ >> testjni(new byte[512]); >> } >> public static void main(String args[]){ >> new test().usejni(); >> } >> } >> --- >> test.h >> --- >> /* DO NOT EDIT THIS FILE - it is machine generated */ >> #include >> /* Header for class test */ >> >> #ifndef _Included_test >> #define _Included_test >> #ifdef __cplusplus >> extern "C" { >> #endif >> /* >> * Class: test >> * Method:testjni >> * Signature: ([B)V >> */ >> JNIEXPORT void JNICALL Java_test_testjni >> (JNIEnv *, jobject, jbyteArray); >> >> #ifdef __cplusplus >> } >> #endif >> #endif >> -- >> test.c >> -- >> #include "test.h" >> JNIEXPORT void JNICALL Java_test_testjni >> (JNIEnv * env, jobject obj,jbyteArray array){ >> jbyte* buf = (jbyte*)malloc(sizeof(jbyte)*512); >> (*env)->GetByteArrayRegion(env,array,512, 0, buf); >> } >> >> (magic number 512 can be any integer, including zero) >> >> > > - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
Jimmy, Jing Lv wrote: > Hi, > > As discussed in the former thread, I find that a JNI of DRLVM > (GetByteArrayRegion) differs from RI in passing parameter > (byte[count],count,0). RI (and J9 VM) returns immediately but DRLVM > throws an ArrayIndexOutOfBoundsException. > IMHO, it is better to improve here, make it follow RI, as JNI is > also a part of API, and users may use it. So if no objections, shall I > or someone raise a JIRA for this? Go ahead and raise a JIRA Jimmy. Regards, Tim > I don't know if Martin is urgent in using Harmony in Winstone > servlet engine (Martin, can you read this?), so shall I work around in > java code before we have conclusion? > > Any comments/suggestions from DRLVM guys? Thanks! > > > Jimmy, Jing Lv wrote: >> Jimmy, Jing Lv wrote: >>> >>> >>> I do some further study and test then, and find the problem was not >>> so easy. >>> >>> Alex and I are correct that "offset <= buffer.length" here is wrong, >>> but the next "count <= buffer.length - offset" seems has proved its >>> correctness. >>> >>> I have a test[1], try to write(new byte[count],count, 0) to a >>> SocketOutputStream, the test passes quietly. I do this test on WinXp >>> Sp2, the latest Harmony workspace, with J9 VM5. >>> >>> I believe the ArrayIndexOutOfBoundsException is throw out when it try >>> to get byte array in the native (GetByteArrayRegion), which is a JNI >>> method. I guess there may be some difference between VMs. >>> >>> Martin, are you using DRLVM? Can someone using DRLVM (or other VMs) >>> run the test below on DRLVM for me? Thanks! >>> >>> [1] >>> public void test_socketOutputStream() throws Exception { >>> ServerSocket ss = new ServerSocket(0); >>> Socket sock = new Socket(); >>> sock.connect(new InetSocketAddress >>>(InetAddress.getLocalHost(),ss.getLocalPort())); >>> ss.accept(); >>> OutputStream os = sock.getOutputStream(); >>> os.write(new byte[0], 0, 0); // passes here >>> os.write(new byte[512], 512, 0); // passes here >>> } >>> >> >> Hi, >> >> At last I have test[1] JNI method(GetByteArrayRegion) of RI >> (before dinner, very hungry... :) ) >> The result is that, RI return successfully if the given offset >> equals length of the byte array, and the given count is zero. >> >> Thus IMHO it is better to improve DRLVM to follow RI, as JNI >> methods are also API methods, users may use it. >> >> Shall we raise a JIRA for DRLVM? Any comments/suggestions from >> DRLVM guys? Thanks! >> >> Time for dinner! :D >> >> [1] >> test.java >> -- >> class test >> { >> static{ >> System.loadLibrary("testjni"); >> } >>private native void testjni(byte[] bs); >> public void usejni(){ >> testjni(new byte[512]); >> } >> public static void main(String args[]){ >> new test().usejni(); >> } >> } >> --- >> test.h >> --- >> /* DO NOT EDIT THIS FILE - it is machine generated */ >> #include >> /* Header for class test */ >> >> #ifndef _Included_test >> #define _Included_test >> #ifdef __cplusplus >> extern "C" { >> #endif >> /* >> * Class: test >> * Method:testjni >> * Signature: ([B)V >> */ >> JNIEXPORT void JNICALL Java_test_testjni >> (JNIEnv *, jobject, jbyteArray); >> >> #ifdef __cplusplus >> } >> #endif >> #endif >> -- >> test.c >> -- >> #include "test.h" >> JNIEXPORT void JNICALL Java_test_testjni >> (JNIEnv * env, jobject obj,jbyteArray array){ >> jbyte* buf = (jbyte*)malloc(sizeof(jbyte)*512); >> (*env)->GetByteArrayRegion(env,array,512, 0, buf); >> } >> >> (magic number 512 can be any integer, including zero) >> >> > > -- Tim Ellison ([EMAIL PROTECTED]) IBM Java technology centre, UK. - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
[DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
Hi, As discussed in the former thread, I find that a JNI of DRLVM (GetByteArrayRegion) differs from RI in passing parameter (byte[count],count,0). RI (and J9 VM) returns immediately but DRLVM throws an ArrayIndexOutOfBoundsException. IMHO, it is better to improve here, make it follow RI, as JNI is also a part of API, and users may use it. So if no objections, shall I or someone raise a JIRA for this? I don't know if Martin is urgent in using Harmony in Winstone servlet engine (Martin, can you read this?), so shall I work around in java code before we have conclusion? Any comments/suggestions from DRLVM guys? Thanks! Jimmy, Jing Lv wrote: Jimmy, Jing Lv wrote: I do some further study and test then, and find the problem was not so easy. Alex and I are correct that "offset <= buffer.length" here is wrong, but the next "count <= buffer.length - offset" seems has proved its correctness. I have a test[1], try to write(new byte[count],count, 0) to a SocketOutputStream, the test passes quietly. I do this test on WinXp Sp2, the latest Harmony workspace, with J9 VM5. I believe the ArrayIndexOutOfBoundsException is throw out when it try to get byte array in the native (GetByteArrayRegion), which is a JNI method. I guess there may be some difference between VMs. Martin, are you using DRLVM? Can someone using DRLVM (or other VMs) run the test below on DRLVM for me? Thanks! [1] public void test_socketOutputStream() throws Exception { ServerSocket ss = new ServerSocket(0); Socket sock = new Socket(); sock.connect(new InetSocketAddress (InetAddress.getLocalHost(),ss.getLocalPort())); ss.accept(); OutputStream os = sock.getOutputStream(); os.write(new byte[0], 0, 0); // passes here os.write(new byte[512], 512, 0); // passes here } Hi, At last I have test[1] JNI method(GetByteArrayRegion) of RI (before dinner, very hungry... :) ) The result is that, RI return successfully if the given offset equals length of the byte array, and the given count is zero. Thus IMHO it is better to improve DRLVM to follow RI, as JNI methods are also API methods, users may use it. Shall we raise a JIRA for DRLVM? Any comments/suggestions from DRLVM guys? Thanks! Time for dinner! :D [1] test.java -- class test { static{ System.loadLibrary("testjni"); } private native void testjni(byte[] bs); public void usejni(){ testjni(new byte[512]); } public static void main(String args[]){ new test().usejni(); } } --- test.h --- /* DO NOT EDIT THIS FILE - it is machine generated */ #include /* Header for class test */ #ifndef _Included_test #define _Included_test #ifdef __cplusplus extern "C" { #endif /* * Class: test * Method:testjni * Signature: ([B)V */ JNIEXPORT void JNICALL Java_test_testjni (JNIEnv *, jobject, jbyteArray); #ifdef __cplusplus } #endif #endif -- test.c -- #include "test.h" JNIEXPORT void JNICALL Java_test_testjni (JNIEnv * env, jobject obj,jbyteArray array){ jbyte* buf = (jbyte*)malloc(sizeof(jbyte)*512); (*env)->GetByteArrayRegion(env,array,512, 0, buf); } (magic number 512 can be any integer, including zero) -- Best Regards! Jimmy, Jing Lv China Software Development Lab, IBM - Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]