RFR(M): 8140594: Various minor code improvements (compiler)

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Mon May 9 10:30:24 UTC 2016


Hi Tobias,

sorry for that!  No, it's not just missing include, windows does not 
implement snprintf before Visual Studio 2015.
http://stackoverflow.com/questions/2915672/snprintf-and-visual-studio-2010

As this is only a build-time tool, I would prefer skipping this altogether
above adding windows specific code:
http://cr.openjdk.java.net/~goetz/wr16/8140594-covCo/webrev.02/

Best regrds,
 Goetz.



> -----Original Message-----
> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
> Sent: Montag, 9. Mai 2016 11:08
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot compiler
> <hotspot-compiler-dev at openjdk.java.net>
> Subject: Re: RFR(M): 8140594: Various minor code improvements (compiler)
> 
> Hi Goetz,
> 
> the build failed on Windows with:
> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(721) :
> error C3861: 'snprintf': identifier not found
> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(723) :
> error C3861: 'snprintf': identifier not found
> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(728) :
> error C3861: 'snprintf': identifier not found
> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(730) :
> error C3861: 'snprintf': identifier not found
> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(740) :
> error C3861: 'snprintf': identifier not found
> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(742) :
> error C3861: 'snprintf': identifier not found
> 
> Looks like a missing include.
> 
> Best regards,
> Tobias
> 
> On 09.05.2016 10:40, Tobias Hartmann wrote:
> > Hi Goetz,
> >
> > thanks for the explanation, this looks good to me!
> >
> > I'm running the hs-comp PIT now and will push the fix afterwards, if there
> are no objections.
> >
> > Thanks,
> > Tobias
> >
> > On 09.05.2016 10:30, Lindenmaier, Goetz wrote:
> >> Hi Tobias,
> >>
> >>> I can sponsor your fix but since we are close to JDK 9 FC, we are not
> allowed
> >>> to push enhancements to hs-comp. Shouldn't this be a "bug" anyway?
> >> Thanks for reviewing and the offer to sponsor!
> >> I changed the Jira issue to bug.  Sorry for being that close to FC with my
> RFR.
> >>
> >>>> - size of pointer passed to jio_snprintf()
> >>> Please also fix the indentation in line 6004.
> >> Fixed.
> >>
> >>>> classLoader.cpp
> >>>> - jio_snprintf does null termination. But it might return -1 if truncated,
> >>>>   in this case array access at -1.
> >>> You can remove "int n;"
> >> Fixed.
> >>
> >>>> generateOopMap.cpp
> >>>> - Remaining fields not initialized.
> >>> I would put the loop bodies in a new line.
> >> Fixed.
> >>
> >>>> relocator.cpp
> >>>> - delta might be -4 ... assert returns.
> >>> I don't understand this change. If delta is -4, the assert in the baseline
> version
> >>> is triggered. With your fix, the assert is triggered as well.
> >>
> >> Sorry, that's a bad comment.  I meant that the assert does not
> >> protect against getting -4 in that line, because coverity knows
> >> about -XX:SuppressErrorAt.  But that's pointless, as assert goes
> >> away in product builds anyways.
> >>
> >> The assert shall be triggered, therefore I moved it out of the if().
> >> But the if() now protects against oob with -4 or smaller.
> >>
> >> I also rebased the change, and added you as reviewer.  Find the new
> >> Webrev here:
> >> http://cr.openjdk.java.net/~goetz/wr16/8140594-covCo/webrev.01/
> >> And the incremental patch:
> >> http://cr.openjdk.java.net/~goetz/wr16/8140594-
> covCo/webrev.01/incremental_diff.patch
> >>
> >> Best regards,
> >>   Goetz.
> >>


More information about the hotspot-compiler-dev mailing list