[9] RFR(S): 8139150: ClassVerifier frees exception message while it's still in use

[harold seigel]

I think we should backport the fix to 8u.

Harold

On 10/8/2015 12:08 PM, Daniel D. Daugherty wrote:
> On 10/8/15 9:38 AM, Tobias Hartmann wrote:
>> Hi,
>>
>> please review the following patch.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8139150
>> http://cr.openjdk.java.net/~thartmann/8139150/webrev.00/
>
> src/share/vm/classfile/stackMapTable.cpp
>     No comments.
>
> Thumbs up!
>
> Did a quick audit and I don't see any other calls to
> class_format_error() with the same issue.
>
> This bug is very old. That ResourceMark came from here:
>
> $ sp -r1.17 src/share/vm/classfile/stackMapTable.cpp
> src/share/vm/classfile/SCCS/s.stackMapTable.cpp:
>
> D 1.17 05/06/20 17:21:50 mingyao 18 17  00015/00008/00446
> MRs:
> COMMENTS:
> Fixed 6275215, VM fails on StackMapTable jcod tests (VerifyError)
> Fixed 6275199, VM fails on StackMapTable jcod tests
> Fixed 6275153, VM fails on StackMapTable tests
>
> And the code looked like this:
>
> 187a185,194
> >     if (offset >= _code_length ||
> >         _code_data[offset] != ClassVerifier::NEW_OFFSET) {
> >       ResourceMark rm(THREAD);
> >       Exceptions::fthrow(
> >         THREAD_AND_LOCATION,
> >         vmSymbolHandles::java_lang_ClassFormatError(),
> >         "StackMapTable format error: bad offset for Uninitialized"
> >       );
> >       return NULL;
> >     }
>
>
> The class_format_error() call came from here:
>
> D 1.21 06/04/13 11:43:50 km88527 23 22  00064/00108/00360
> MRs:
> COMMENTS:
> fixed 6402717: Error verifying java.lang.Error causes VM to exit 
> silently due to stack overflow
>
> and the code changed to look like this:
>
>     if (offset >= _code_length ||
>         _code_data[offset] != ClassVerifier::NEW_OFFSET) {
>       ResourceMark rm(THREAD);
>       _verifier->class_format_error(
>         "StackMapTable format error: bad offset for Uninitialized");
>       return NULL;
>     }
>
> This fix should probably be backported... but I would
> check with Harold...
>
> Dan
>
>
>>
>> Problem:
>> If class verification fails in 
>> StackMapReader::parse_verification_type(), 
>> ClassVerifier::class_format_error() is invoked to pass the error 
>> message. The method allocates a new string and saves it in 
>> ClassVerifier::_message. The problem is that the caller creates a new 
>> ResourceMark that leads to _message being deallocated after return. 
>> However, later in Verifier::verify() we call 
>> ClassVerifier::exception_message() to get the message and pass it on.
>>
>> Solution:
>> We should not create a ResourceMark here. There is a top level 
>> ResourceMark in Verifier::verify() that will take care of freeing the 
>> memory allocated in ClassVerifier::class_format_error().
>>
>> Tested with JPRT and the failing testcase.
>>
>> Thanks,
>> Tobias
>