Question on ADLC's RegClass destructor

Zoltán Majó zoltan.majo at oracle.com
Wed May 27 08:09:50 UTC 2015


Hi Kris,



On 05/26/2015 09:34 PM, Krystal Mok wrote:
> Hi Zoltan,
>
> Thanks for your reply. When I was looking at the call chain that leads 
> to RegClass' constructor, get_ident() is what's passed into RegClass 
> as classid.
>
> The comment on get_ident_common() says:
> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/ac291bc3ece2/src/share/vm/adlc/adlparse.cpp#l4560
> //------------------------------get_ident_common-------------------------------
> // Looks for an identifier in the buffer, and turns it into a null 
> terminated
> // string(still inside the file buffer).  Returns a pointer to the 
> string or
> // NULL if some other token is found instead.
> char *ADLParser::get_ident_common(bool do_preproc) {
>
> So normally, the string returned should be still inside the file 
> buffer (if no preprocessing is needed...), and shouldn't need to be 
> free'd afterwards; but if preprocessing is needed, then yes, there's 
> dynamically allocated memory for the string returned.

thank you for the detailed explanation and for bringing attention to 
this issue!

I filed JDK-8081288 [1] to track the problem and will take care of it 
hopefully soon. If you have some time to spare, you are of course 
welcome to contribute a patch.

Best regards,


Zoltan

[1] https://bugs.openjdk.java.net/browse/JDK-8081288

> get_ident() is get_ident_common(true), so it's possible that 
> preprocessing is needed; but from the RegClass constructor, it'd be 
> hard to tell whether the string passed in is from the file buffer or 
> from a dynamically allocated piece of memory.
>
> It (the original code before your changes) was an odd piece of code to 
> begin with...
>
> - Kris
>
>
> On Tue, May 26, 2015 at 12:06 AM, Zoltán Majó <zoltan.majo at oracle.com 
> <mailto:zoltan.majo at oracle.com>> wrote:
>
>     Hi Kris,
>
>
>     On 05/23/2015 12:22 AM, Krystal Mok wrote:
>
>         Hi compiler team,
>
>         I came across this piece of code in ADLC, and I'm not sure if
>         it's right or not:
>
>         http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/ac291bc3ece2/src/share/vm/adlc/formsopt.cpp#l236
>
>         RegClass::~RegClass() {
>           delete _classid;
>         }
>
>         Why do we need to delete the _classid field here? If this is
>         needed, then a bunch of other classes in formsopt.hpp would
>         need the same treatment.
>
>
>     If I remember correctly, I added the destructor because the caller
>     of the RegClass constructor allocates memory for _classid, but
>     does not free it. But I'd have to check to be sure.
>
>     Best regards,
>
>
>     Zoltan
>
>
>
>         (I know this virtual destructor was added mainly for the two
>         new subclasses, but I'm just wondering about this one...)
>
>         Thanks,
>         Kris
>
>
>



More information about the hotspot-compiler-dev mailing list