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

Thomas Stüfe thomas.stuefe at gmail.com
Mon Oct 12 14:45:03 UTC 2015


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.
>
>
>
>


More information about the hotspot-runtime-dev mailing list