RFR(M): 8139040: Fix initializations before ShouldNotReachHere()

[Thomas Stüfe]

Hi Goetz,

thanks! looks good, nothing further to add from me.

Regards, Thomas

On Tue, Oct 13, 2015 at 8:52 AM, Lindenmaier, Goetz <
goetz.lindenmaier at sap.com> wrote:

> Hi Thomas,
>
>
>
> thanks for looking at this!
>
>
>
> C1_LIRGenerator.hpp:
>
> I added handling of the default case (assert(0)).
>
>
>
> compactHashtable.cpp:
>
> Hmm, I’m not clear why get_num says corrupted() in one case,
>
> and in the other returns false.  The code is dead, anyways, so I
>
> rather leave it as is.
>
>
>
> heapRegionRemSet.cpp
>
> I added guarantee(max_prev != NULL).  I think this is better
> understandable.
>
> Else it looks as if there is a legal case where *max_prev must not be set.
>
>
>
> Updated webrev:
>
> http://cr.openjdk.java.net/~goetz/webrevs/8139040-init/webrev.03/
>
>
>
> Best regards,
>
>   Goetz.
>
>
>
>
>
>
>
>
>
> *From:* Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
> *Sent:* Montag, 12. Oktober 2015 16:45
> *To:* Lindenmaier, Goetz
> *Cc:* hotspot-runtime-dev at openjdk.java.net
> *Subject:* Re: RFR(M): 8139040: Fix initializations before
> ShouldNotReachHere()
>
>
>
> Hi Goetz,
>
>
>
> More small nitpicks:
>
>
>
>
> http://cr.openjdk.java.net/~goetz/webrevs/8139040-init/webrev.02/src/share/vm/c1/c1_LIRGenerator.hpp.sdiff.html
>
>
>
> handle default case? I dont see callers handling lir_cond_unknown.
>
>
>
> ------------
>
>
>
>
> http://cr.openjdk.java.net/~goetz/webrevs/8139040-init/webrev.02/src/share/vm/classfile/compactHashtable.cpp.html
>
>
>
> maybe the right thing to do would be to handle errors returned by
> HashTableHexDump::get_num() ?
>
> (but your change is still better than the original)
>
>
>
> --------------
>
>
>
>
> http://cr.openjdk.java.net/~goetz/webrevs/8139040-init/webrev.02/src/share/vm/gc/g1/heapRegionRemSet.cpp.sdiff.html
>
>
>
> In PerRegionTable* OtherRegionsTable::delete_region_table():
>
>
>
>  617   // Unsplice.
>
>  618   *max_prev = max->collision_list_next();
>
>
>
> Should the access to max_prev be guarded against != NULL?
>
>
>
> ------------
>
>
>
> Otherwise the change looks good.
>
>
>
> Kind Regards, Thomas
>
>
>
>
>
> On Mon, Oct 12, 2015 at 11:18 AM, Lindenmaier, Goetz <
> goetz.lindenmaier at sap.com> wrote:
>
> Hi,
>
> I now fixed the warnings showing with -Wuninitialized anyways.
> The change now enables this flag per default with gcc 4.8.
> Older gcc versions detect more issues, as far as I checked all false
> positives. Therefore I don't enable the flag for gcc < 4.8.
>
> The warnings reported by this flag depend on the optimization level, it's
> most effective in the opt build.
>
> This unveiled some missing initializations in constructors, as in
> compile.hpp,
> and oopMap.hpp that can actually cause wrong behavior.
>
> Complete webrev:
> http://cr.openjdk.java.net/~goetz/webrevs/8139040-init/webrev.02/
> incremental webrev:
>
> http://cr.openjdk.java.net/~goetz/webrevs/8139040-init/webrev.02-incremental/
>
> Best regards,
>   Goetz.
>
>
> From: Lindenmaier, Goetz
> Sent: Mittwoch, 7. Oktober 2015 15:22
> To: hotspot-runtime-dev at openjdk.java.net
> Subject: RFR(M): 8139040: Fix initializations before ShouldNotReachHere()
>
>
> Hi,
>
> SAP requires us to fix a row of issues in the hotspot coding.  I would like
> to share these with openJDK.
>
> This webrev fixes a row of missing intializations, mostly combined with
> ShouldNotReachHere()
> in default cases of switches or the like.
> http://cr.openjdk.java.net/~goetz/webrevs/8139040-init/webrev.00/
>
> In the debug build, ShouldNotReachHere() can be suppressed, so the
> uninitialized value actually can cause problems.
> In opt builds, not all tools recognize the ShouldNotReachHere properly.
>
> In addition to this I would like to add -Wuninitialized to the warning
> flags.
> This finds most of these issues in the opt build and
> would require an additional 70 fixes plus fixes in jvmtiEnter.xsl.
> Would it be appreciated to set this flag?
>
> Best regards,
>   Goetz.
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151013/6e69fed5/attachment-0001.html>