(jdk10) RFR(xxs): 8176872: [s390] wrong pc shown in error logs

[David Holmes]

Hi Thomas,

On 23/03/2017 10:58 PM, Thomas Stüfe wrote:
> Hi,
>
> a second
> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.01/webrev/
>
> I forgot that the pc inside the secondary signal handler for crashes
> during error reporting needs to be adjusted too.

Ah - so no longer a s390-only fix. :(  Will need to consider whether we 
should update to always access si_addr.

David

> Also, I need a second reviewer.
>
> Thanks, Thomas
>
> On Wed, Mar 22, 2017 at 9:05 AM, Thomas Stüfe <thomas.stuefe at gmail.com
> <mailto:thomas.stuefe at gmail.com>> wrote:
>
>     Hi David, thanks for the review! Let's keep the s390-only solution
>     for now. Cleanup can be done in a separate fix.
>
>     Kind Regards, Thomas
>
>     On Wed, Mar 22, 2017 at 12:47 AM, David Holmes
>     <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>
>         Hi Thomas,
>
>         On 21/03/2017 11:40 PM, Thomas Stüfe wrote:
>
>             Hi all,
>
>             please take a look at this tiny fix. It fixes the pc shown
>             as faulting
>             address for SIGILL and SIGFPE in hs_err files.
>
>             https://bugs.openjdk.java.net/browse/JDK-8176872
>             <https://bugs.openjdk.java.net/browse/JDK-8176872>
>             http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.00/webrev/
>             <http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.00/webrev/>
>
>             When determining the crash pc, in all posix platform signal
>             handlers pc is
>             taken from the context. However, context.pc on zlinux points
>             to the
>             instruction *after* the faulting op. The correct way,
>             according to POSIX,
>             would be to take the address from siginfo_t->si_addr for
>             signals SIGILL,
>             SIGFPE.
>
>
>         Well si_addr is the platform independent POSIX way of getting
>         the address. Getting it from the uc_mcontext is architecture
>         specific and seemingly not specified as to what the "pc"
>         actually contains - the faulting instruction or the next
>         instruction. If it were specified then the bug might be in zlinux :)
>
>             (actually, this patch would make sense for all POSIX
>             platforms, but only
>             s390 seems to show this error, so I leave the patch local to
>             s390.)
>
>
>         I think it is historical that we get the PC from the uc_mcontext
>         because originally we did not have SA_SIGINFO available. So a
>         broader cleanup of this code is certainly possible. But for now
>         this S390 fix seems fine.
>
>         Thanks,
>         David
>         -----
>
>             Kind Regards, Thomas
>
>
>