RFR(S): 8040920: Uninitialised memory in hotspot/src/share/vm/code/dependencies.cpp

[Vladimir Kozlov]

On 8/5/14 10:28 AM, Morris Meyer wrote:
> Vladimir,
>
> Here is a revised version with your suggestions.
>
> I added outer ResourceMarks to
>
>   void Dependencies::log_all_dependencies()
>
> and
>
> void Dependencies::write_dependency_to(CompileLog* log,
>                                         DepType dept,
> GrowableArray<DepArgument>* args,
>                                         Klass* witness)
>
> as they are the only callers to

No, it is also called from log_dependency() (the one in 
dependencies.hpp). That is why asked about missed RM in it.

>
> void Dependencies::write_dependency_to(CompileLog* log,
>                                         DepType dept,
> GrowableArray<ciBaseObject*>* args,
>                                         Klass* witness);
>
> which does not have a ResourceMark - so these aren't nested cases.
>
> void Dependencies::DepStream::log_dependency(Klass* witness)
>
> does call
>
> void Dependencies::write_dependency_to(CompileLog* log,
>                                         DepType dept,
> GrowableArray<DepArgument>* args,
>                                         Klass* witness)
>
> which makes it a nested usage of ResourceMark - and I've added a
> guarantee() to ensure the outer ResourceMark is not affected by the
> inner callee.
>
> void Dependencies::DepStream::print_dependency(Klass* witness, bool
> verbose);
>
> has a new GrowableArray usage and I also added an outer ResourceMark
> here - and a guarantee() that goes along with the inner ResourceMark
> used in the existing code of
>
> void Dependencies::print_dependency(DepType dept,
> GrowableArray<DepArgument>* args,
>                                      Klass* witness);

New guarantees are fine.

Thanks,
Vladimir

>
> This revision has been tested with JPRT.
>
> Thanks for the review!
>
>          --morris
>
> JBS - https://bug.openjdk.java.net/browse/JDK-8040920
> WEBREV - http://cr.openjdk.java.net/~morris/JDK-8040920.08
>
> On 7/22/14, 7:05 PM, Vladimir Kozlov wrote:
>> how you determined where you should place ResourceMark? You missed one
>> in log_dependency().
>>
>> write_dependency_to() methods are used only by Dependencies so they
>> could be private. Then we would guarantee that they are called in
>> known places which may have ResourceMark already.
>>
>> thanks,
>> Vladimir
>>
>> On 7/21/14 3:42 PM, Morris Meyer wrote:
>>> Christian and Vladimir,
>>>
>>> I have incorporated these comments into this latest webrev.  It has been
>>> put through JPRT west.
>>>
>>>          --mm
>>>
>>> WEBREV - http://cr.openjdk.java.net/~morris/JDK-8040920.07
>>> JBS - https://bug.openjdk.java.net/browse/JDK-8040920
>>>
>>>
>>> On 4/30/14, 6:41 PM, Christian Thalinger wrote:
>>>> On Apr 30, 2014, at 11:55 AM, Vladimir Kozlov
>>>> <vladimir.kozlov at oracle.com> wrote:
>>>>
>>>>> On 4/30/14 2:35 PM, Christian Thalinger wrote:
>>>>>> On Apr 30, 2014, at 11:26 AM, Vladimir Kozlov
>>>>>> <vladimir.kozlov at oracle.com> wrote:
>>>>>>
>>>>>>> Morris,
>>>>>>>
>>>>>>> How you verified that you put ResourceMark in all needed places?
>>>>>>> That is what I don't like about GrowableArray.
>>>>>> He could put a ResourceMark in front of every GrowableArray.  That
>>>>>> would do it.
>>>>> Nested ResourceMark has bad effect on external GrowableArray: if it
>>>>> grows after inner ResourceMark that space will be released after
>>>>> inner scope ends. You will get memory stomps because of that. You
>>>>> need to be sure that external GrowableArray never grows inside inner
>>>>> ResourceMark scope.
>>>> Ugh.  You’re right.  Yeah, as I said we don’t need a GrowableArray
>>>> here but the regular Array is only for metadata.
>>>>
>>>>> Vladimir
>>>>>
>>>>>>> Thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 4/30/14 11:47 AM, Morris Meyer wrote:
>>>>>>>> Vladimir and Christian,
>>>>>>>>
>>>>>>>> Thank you for the reviews.
>>>>>>>>
>>>>>>>> This version has your comments incorporated and has been put
>>>>>>>> through
>>>>>>>> JPRT and passes Parfait cleanly.
>>>>>>>>
>>>>>>>>          --mm
>>>>>>>>
>>>>>>>> JBS - https://bugs.openjdk.java.net/browse/JDK-8040920
>>>>>>>> WEBREV - http://cr.openjdk.java.net/~morris/JDK-8040920.05
>>>>>>>>
>>>>>>>>
>>>>>>>> On 4/24/14, 2:16 PM, Vladimir Kozlov wrote:
>>>>>>>>> Using GrowableArray requires adding ResourceMark otherwise it
>>>>>>>>> will be
>>>>>>>>> memory leak. Method clear() does not free resources.
>>>>>>>>>
>>>>>>>>> Vladimir
>>>>>>>>>
>>>>>>>>> On 4/24/14 9:30 AM, Morris Meyer wrote:
>>>>>>>>>> Folks,
>>>>>>>>>>
>>>>>>>>>> Could I get a review for this parfait issue?  I've refactored
>>>>>>>>>> dependencies to use a GrowableArray to protect from
>>>>>>>>>> inconsistencies in
>>>>>>>>>> passed in argument pairs (int nargs, DepArgument args[])
>>>>>>>>>>
>>>>>>>>>> Thanks to Christian Thalinger and Vladimir Kozlov for looking at
>>>>>>>>>> earlier
>>>>>>>>>> versions of this.
>>>>>>>>>>
>>>>>>>>>>          --morris
>>>>>>>>>>
>>>>>>>>>> JBS - https://bugs.openjdk.java.net/browse/JDK-8040920
>>>>>>>>>> WEBREV - http://cr.openjdk.java.net/~morris/JDK-8040920.04
>>>>>>>>>>
>>>
>