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