(jdk10) RFR(xxs): 8176872: [s390] wrong pc shown in error logs
David Holmes
david.holmes at oracle.com
Tue Apr 4 20:33:09 UTC 2017
On 5/04/2017 12:03 AM, Thomas Stüfe wrote:
> Hi David,
>
> what a stupid bug! I think I must have accidentally tested an older version.
>
> // support safefetch faults in error handling
> ucontext_t* const uc = (ucontext_t*) ucVoid;
> - address pc = (uc == NULL) ? os::Posix::ucontext_get_pc(uc) : NULL;
> + address pc = (uc != NULL) ? os::Posix::ucontext_get_pc(uc) : NULL;
>
> :-/
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.03/webrev/
Ouch! Three sets of eyes missed that :(
Pushing changes now.
Thanks,
David
> Kind Regards, Thomas
>
>
> On Tue, Apr 4, 2017 at 2:54 PM, David Holmes <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>> wrote:
>
> Sorry Thomas but I'm seeing a test failure on linux-x64 in the
> runtime/ErrorHandling/SafeFetchInErrorHandlingTest
>
> Seems safefetch is not working:
>
> #
> # A fatal error has been detected by the Java Runtime Environment:
> Will test SafeFetch...
>
> [error occurred during error reporting (test safefetch in error
> handler), id 0xb]
>
> #
> # SIGSEGV (0xb) at pc=0x00007f078dd58582, pid=603, tid=604
>
>
> I don't see this with a JDK9 build. Can you test it?
>
> I have to call it a night now.
>
> David
>
>
> On 4/04/2017 10:20 PM, David Holmes wrote:
>
> Hi Thomas,
>
> Yes I can sponsor this.
>
> Cheers,
> David
>
> On 4/04/2017 10:16 PM, Thomas Stüfe wrote:
>
>
>
> On Tue, Apr 4, 2017 at 12:04 PM, David Holmes
> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
> <mailto:david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>>> wrote:
>
> On 4/04/2017 5:32 PM, Thomas Stüfe wrote:
>
> Hi David,
>
> On Tue, Apr 4, 2017 at 9:25 AM, David Holmes
> <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>
> <mailto:david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>>
> <mailto:david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>
> <mailto:david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>>>> wrote:
>
> Hi Thomas,
>
> On 4/04/2017 3:54 AM, Thomas Stüfe wrote:
>
> Hi Dmitry, David,
>
> sorry for dropping the ball on this one.
> Here the latest
> version - I
> only fixed the cosmetics Dmitry requested:
>
>
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.02/webrev/
> <http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.02/webrev/>
>
>
> <http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.02/webrev/
> <http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.02/webrev/>>
>
>
>
> <http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.02/webrev/
> <http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.02/webrev/>
>
>
> <http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.02/webrev/
> <http://cr.openjdk.java.net/~stuefe/webrevs/8176872-s390-wrong-pc-in-errorlogs/jdk10-webrev.02/webrev/>>>
>
>
>
> Looks okay. I'm a little unsure about only
> fixing this in
> the shared
> code for non-s390. But I guess we can deal with
> other
> platforms later.
>
>
> Not sure what you mean, but a common solution for
> all posix
> platforms
> would be even easier. Instead of fixing the s390
> signal handler
> and the
> secondary signal handler for all posix platforms,
> one could
> adjust the
> pc at the entry of error reporting, in
> VMError::report_and_die(). But
> that would require an #ifndef WIN32 and you guys
> frown upon
> ifdefs, so
> that is why I avoided this solution.
>
>
> Well there are frowns and there are frowns :) I don't
> like that the
> other platforms don't use si_addr too - seems the
> "right" way to get
> the pc and what would be done if writing the signal
> handler today.
> But then I also don't understand why handle_linux_signal is
> duplicated across CPU specific files! Maybe there are
> some subtle
> differences but it should be pretty OS specific rather
> than CPU
> specific.
>
>
> I agree. Would be a good subject for cleanup.
>
>
>
> Anyway ...
>
> But lets commit this solution, and if other
> platforms should
> ever have
> the same problem, we can modify the code.
>
>
> ... lets proceed as is.
>
>
> Thanks! Could you please sponsor this?
>
> Thanks, Thomas
>
>
>
> Thanks,
> David
>
> ..Thomas
>
>
> Thanks,
> David
> -----
>
> Thanks for reviewing!
>
> Kind Regards, Thomas
>
> On Sun, Mar 26, 2017 at 12:22 PM, Dmitry
> Samersoff
> <dmitry.samersoff at oracle.com
> <mailto:dmitry.samersoff at oracle.com>
> <mailto:dmitry.samersoff at oracle.com
> <mailto:dmitry.samersoff at oracle.com>>
> <mailto:dmitry.samersoff at oracle.com
> <mailto:dmitry.samersoff at oracle.com>
> <mailto:dmitry.samersoff at oracle.com
> <mailto:dmitry.samersoff at oracle.com>>>
> <mailto:dmitry.samersoff at oracle.com
> <mailto:dmitry.samersoff at oracle.com>
> <mailto:dmitry.samersoff at oracle.com
> <mailto:dmitry.samersoff at oracle.com>>
>
> <mailto:dmitry.samersoff at oracle.com
> <mailto:dmitry.samersoff at oracle.com>
> <mailto:dmitry.samersoff at oracle.com
> <mailto:dmitry.samersoff at oracle.com>>>>> wrote:
>
> Thomas,
>
> Looks good to me,
>
> We may consider to always use info->si_addr.
>
> Nits:
>
> vmError_posix.cpp
>
> 118:
> Please change uc ? ... to (uc ==
> NULL) ? ...
>
> 122: (and os_linux_s390.cpp:513)
>
> Space missed after (address) ...
>
>
> -Dmitry
>
>
> On 2017-03-21 16:40, 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>
> <https://bugs.openjdk.java.net/browse/JDK-8176872
> <https://bugs.openjdk.java.net/browse/JDK-8176872>>
>
> <https://bugs.openjdk.java.net/browse/JDK-8176872
> <https://bugs.openjdk.java.net/browse/JDK-8176872>
> <https://bugs.openjdk.java.net/browse/JDK-8176872
> <https://bugs.openjdk.java.net/browse/JDK-8176872>>>
>
> <https://bugs.openjdk.java.net/browse/JDK-8176872
> <https://bugs.openjdk.java.net/browse/JDK-8176872>
> <https://bugs.openjdk.java.net/browse/JDK-8176872
> <https://bugs.openjdk.java.net/browse/JDK-8176872>>
>
> <https://bugs.openjdk.java.net/browse/JDK-8176872
> <https://bugs.openjdk.java.net/browse/JDK-8176872>
> <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/>
>
>
> <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/>>
>
>
>
> <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/>
>
>
> <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/>>>
>
>
>
>
> <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/>
>
>
> <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/>>
>
>
>
> <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/>
>
>
> <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.
> >
> > (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.)
> >
> > Kind Regards, Thomas
> >
>
>
> --
> Dmitry Samersoff
> Oracle Java development team, Saint
> Petersburg,
> Russia
> * I would love to change the world, but
> they won't
> give me
> the sources.
>
>
>
>
>
More information about the s390x-port-dev
mailing list