[9] RFR(L) 8158168: SIGSEGV: CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8 (original) (raw)

dean.long at oracle.com dean.long at oracle.com
Fri Mar 17 20🔞49 UTC 2017

On 3/17/17 5:58 AM, Vladimir Ivanov wrote:

I have the same concern. Can we fix the immediate problem in 9 and integrate verification logic in 10?

OK, Tobias is suggesting having verification logic only inside the intrinsics. Are you suggesting removing that as well? Yes and put them back in 10.

OK.

I'm OK with removing all the verification, but that won't reduce the library changes much. I could undo the renaming to Trusted.getChar, but we would still have the bounds checks moved into StringUTF16. I suggest to go with a point fix for 9: just add missing range checks. Is AbstractStringBuilder.append() the only affected method? (Sorry, it's hard to say exactly where the problem is by looking at the diff.)

In the failing test, yes, it was append, but when I went to fix the problem I found that it was much more wide spread, and there were several methods that were affected.

I really like the refactoring you propose on jdk side, but there are pieces I'm not sure about. For example, I spotted a repeated range check:

jdk/src/java.base/share/classes/java/lang/AbstractStringBuilder.java: public void setCharAt(int index, char ch) { checkIndex(index, count); if (isLatin1() && StringLatin1.canEncode(ch)) { value[index] = (byte)ch; } else { if (isLatin1()) { inflate(); } StringUTF16.putCharSB(value, index, ch); } }

OK, I did not look for redundant checks. This check is actually not redundant. The "value" array may be oversized, so "count" is supposed to contain the current maximum. For the safety of the intrinsic array access, we check against the array length, but for the API we need to be stricter and check against the character count.

However, the checkIndex() here is a good example of what is wrong. Let's say we were checking against value.length instead of "count". Even if checkIndex() succeeds here, based on the current length of "value", we can't trust it because the object is mutable and "value" can change between the checkIndex() and putCharSB().

dl

Best regards, Vladimir Ivanov

More information about the hotspot-dev mailing list