RFR (XS): 8144881: Various fixes to linux/sparc

[Volker Simonis]

On Tue, Dec 8, 2015 at 7:13 PM, Mikael Vidstedt
<mikael.vidstedt at oracle.com> wrote:
>
> On Dec 8, 2015, at 01:42, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>
> Hi guys,
>
> On Tue, Dec 8, 2015 at 8:58 AM, Volker Simonis <volker.simonis at gmail.com>
> wrote:
>>
>> Hi Mikael,
>>
>> as David mentioned, your change should be synchronized with Thomas'
>> change, who is currently cleaning up os::print_siginfo() (see:
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2015-December/017012.html).
>>
>
> Thanks for considering me! But I think this does not conflict with my
> change. Even if it does I can merge Mikaels changes in.
>
>
> Thanks, much appreciated.
>
>
>>
>> Regarding your change, I can't understand the reason for including
>> code/codeCache.hpp into all os_cpu/vm/os_<os>_<cpu>.cpp files. These
>> files compiled fine until now, so why unnecessarily adding another
>> dependency?
>>
>
> That would be my question too.
>
>
> I can go either way on this one, really no strong opinions.
>
> The fact is that the files in question *do* make use of CodeCache without
> including the codeCache.hpp. I can only assume that in the other cases
> codeCache.hpp gets included implicitly - either through precompiled.hpp or
> through some other header file. I found at least one case where it wasn't
> included at all, and that resulted in a build error.
>
> For correctness and consistency reasons it could be argued that
> codeCache.hpp really should be included where it's logic is used, but as I
> said I really don't have a strong opinion. At all.
>

OK, you're right. I wasn't clear to me before but CodeCache is indeed
used in the corresponding files.

In that case your change is good - please leave it as it is.

Regards,
Volker

> Cheers,
> Mikael
>
>
>
> Kind Regards, Thomas
>
>>
>> Regards,
>> Volker
>>
>>
>> On Tue, Dec 8, 2015 at 6:21 AM, David Holmes <david.holmes at oracle.com>
>> wrote:
>> > Mikael,
>> >
>> > I'll take a look at this asap but it likely conflicts with other changes
>> > that Thomas Stufe has out for review wrt the signal handling code.
>> >
>> > I'm somewhat suspicious of the sudden need to change linux code just
>> > because
>> > of the sparc port, however I shall reserve judgement for now :)
>> >
>> > Thanks,
>> > David
>> >
>> >
>> >
>> > On 8/12/2015 9:16 AM, Mikael Vidstedt wrote:
>> >>
>> >>
>> >> Good control question! Since these are build problems, and since
>> >> building on other platforms works today, making corresponding changes
>> >> to
>> >> files for other platforms is not strictly necessary, but for
>> >> consistency
>> >> it's of course reasonable to do so anyway. Here's an updated webrev
>> >> doing exactly that:
>> >>
>> >> http://cr.openjdk.java.net/~mikael/webrevs/8144881/webrev.01/webrev/
>> >>
>> >> Changes since webrev.00:
>> >>
>> >> * Copyright year updated (*grumble*)
>> >> * Changes to print sigflags in check_signal_handler in os_linux.cpp
>> >> were
>> >> also applied to the other os_*.cpp files (where applicable)
>> >> * code/codeCache.hpp included in all os_cpu/vm/os_<os>_<cpu>.cpp files
>> >> (where applicable)
>> >> * Added return statement to other JVM_handle_*_signal (where
>> >> applicable)
>> >> - these functions could *really* use some additional
>> >> cleanup/unification...
>> >> * Also: I noticed that the rest of the code in os_linux_sparc.cpp uses
>> >> "sigcontext", as opposed to "struct sigcontext", so I dropped the
>> >> "struct"
>> >>
>> >> Note: I'll have to build this again to make sure it still builds
>> >> correctly across all the platforms, so consider this an optimistic
>> >> webrev+review.
>> >>
>> >> Cheers,
>> >> Mikael
>> >>
>> >>
>> >> On 2015-12-07 14:19, Dmitry Samersoff wrote:
>> >>>
>> >>> Mikael,
>> >>>
>> >>> The changes looks good for me, but
>> >>>
>> >>> Most of changes don't look as sparc-specific.
>> >>>
>> >>> Do you need to update other platforms as well?
>> >>>
>> >>> -Dmitry
>> >>>
>> >>> On 2015-12-08 00:53, Mikael Vidstedt wrote:
>> >>>>
>> >>>> Please review this small change which fixes a handful of places which
>> >>>> currently lead to build failures on linux/sparc. The changes are all
>> >>>> relatively straightforward and come in a few different flavors:
>> >>>>
>> >>>> * Incorrect format specifiers
>> >>>> * "reaches end of function returning value"
>> >>>> * wrong type name (sigcontext_t -> struct sigcontext)
>> >>>> * wrong variable name (addr -> pc)
>> >>>>
>> >>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8144881
>> >>>> Webrev:
>> >>>> http://cr.openjdk.java.net/~mikael/webrevs/8144881/webrev.00/webrev/
>> >>>>
>> >>>> Cheers,
>> >>>> Mikael
>> >>>>
>> >>>
>> >>
>> >
>
>