Re: RFR: 8297530: java.lang.IllegalArgumentException: Negative length on strings concatenation [v2]
On Thu, 24 Nov 2022 17:50:42 GMT, Andrey Turbanov wrote:
>> Claes Redestad has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Require 64-bit arch to avoid (most) hypothetical false positives
>
> test/jdk/java/lang/String/concat/ImplicitStringConcatOOME.java line 129:
>
>> 127: }
>> 128:
>> 129: public static void test(String expected, String actual) {
>
> When this method is called?
Never - accidental left-over from the test I used as a template for this. I'd
remove it if I hadn't already integrated - but opening a bug to clean that out
doesn't seem worthwhile.
-
PR: https://git.openjdk.org/jdk/pull/11354
Re: RFR: 8297530: java.lang.IllegalArgumentException: Negative length on strings concatenation [v2]
On Thu, 24 Nov 2022 15:01:12 GMT, Claes Redestad wrote:
>> When concatenating Strings, OutOfMemoryError should be thrown on all
>> overflow conditions. This fixes a case that erroneously thro IAE on
>> concatenations of long (`length > Integer.MAX_VALUE/2`) UTF16 strings due
>> failing to check for overflow after shifting index left with the coder.
>>
>> This was caught by a fuzzer test. Added a sanity test that fails without the
>> patch (loosely derived from ImplicitStringConcatMany.java)
>
> Claes Redestad has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Require 64-bit arch to avoid (most) hypothetical false positives
test/jdk/java/lang/String/concat/ImplicitStringConcatOOME.java line 129:
> 127: }
> 128:
> 129: public static void test(String expected, String actual) {
When this method is called?
-
PR: https://git.openjdk.org/jdk/pull/11354
Re: RFR: 8297530: java.lang.IllegalArgumentException: Negative length on strings concatenation [v2]
On Thu, 24 Nov 2022 15:01:12 GMT, Claes Redestad wrote: >> When concatenating Strings, OutOfMemoryError should be thrown on all >> overflow conditions. This fixes a case that erroneously thro IAE on >> concatenations of long (`length > Integer.MAX_VALUE/2`) UTF16 strings due >> failing to check for overflow after shifting index left with the coder. >> >> This was caught by a fuzzer test. Added a sanity test that fails without the >> patch (loosely derived from ImplicitStringConcatMany.java) > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Require 64-bit arch to avoid (most) hypothetical false positives Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/11354
Re: RFR: 8297530: java.lang.IllegalArgumentException: Negative length on strings concatenation [v2]
On Thu, 24 Nov 2022 14:14:56 GMT, Alan Bateman wrote: >> Claes Redestad has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Require 64-bit arch to avoid (most) hypothetical false positives > > test/jdk/java/lang/String/concat/ImplicitStringConcatOOME.java line 36: > >> 34: * @run main/othervm -Xverify:all -Xmx4g ImplicitStringConcatOOME >> 35: * >> 36: * @compile ImplicitStringConcatOOME.java > > I don't think you need the @compile tags but you might need `@requires > sun.arch.data.model == "64"` so that the test only runs on 64-bit systems. Seems to work without yes. Started with a copy of a similar test that used the `@compile` tags to control different compilation modes. Testing each ISC compilation mode seemed excessive for this sanity test. - PR: https://git.openjdk.org/jdk/pull/11354
Re: RFR: 8297530: java.lang.IllegalArgumentException: Negative length on strings concatenation [v2]
> When concatenating Strings, OutOfMemoryError should be thrown on all overflow > conditions. This fixes a case that erroneously thro IAE on concatenations of > long (`length > Integer.MAX_VALUE/2`) UTF16 strings due failing to check for > overflow after shifting index left with the coder. > > This was caught by a fuzzer test. Added a sanity test that fails without the > patch (loosely derived from ImplicitStringConcatMany.java) Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Require 64-bit arch to avoid (most) hypothetical false positives - Changes: - all: https://git.openjdk.org/jdk/pull/11354/files - new: https://git.openjdk.org/jdk/pull/11354/files/884dac30..124a7350 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11354&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11354&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11354.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11354/head:pull/11354 PR: https://git.openjdk.org/jdk/pull/11354
