RFR: 8205199: more Linux clang compile failures

Martin Buchholz martinrb at google.com
Tue Jun 19 20:07:17 UTC 2018


Thomas: Thanks.  Webrev updated.


8205199: more Linux clang compile failures
http://cr.openjdk.java.net/~martin/webrevs/jdk/more-linux-clang-failures/
https://bugs.openjdk.java.net/browse/JDK-8205199


On Tue, Jun 19, 2018 at 12:50 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
wrote:

> Hi all,
>
> debug.cpp: I'm sorry for the bug. That was not my best day, apparently.
>
> Please use the following patch, which just converts the context-store
> function to a void().
>
> diff -r 770fcde6a437 src/hotspot/share/utilities/debug.cpp
> --- a/src/hotspot/share/utilities/debug.cpp     Tue Jun 19 11:08:14 2018
> +0200
> +++ b/src/hotspot/share/utilities/debug.cpp     Tue Jun 19 21:46:59 2018
> +0200
> @@ -714,16 +714,13 @@
>    }
>  }
>
> -static bool store_context(const void* context) {
> -  if (memcpy(&g_stored_assertion_context, context,
> sizeof(ucontext_t)) == false) {
> -    return false;
> -  }
> +static void store_context(const void* context) {
> +  memcpy(&g_stored_assertion_context, context, sizeof(ucontext_t));
>  #if defined(__linux) && defined(PPC64)
>    // on Linux ppc64, ucontext_t contains pointers into itself which
> have to be patched up
>    //  after copying the context (see comment in sys/ucontext.h):
>    *((void**) &g_stored_assertion_context.uc_mcontext.regs) =
> &(g_stored_assertion_context.uc_mcontext.gp_regs);
>  #endif
> -  return true;
>  }
>
>  bool handle_assert_poison_fault(const void* ucVoid, const void*
> faulting_address) {
> @@ -734,9 +731,8 @@
>      if (ucVoid) {
>        const intx my_tid = os::current_thread_id();
>        if (Atomic::cmpxchg(my_tid, &g_asserting_thread, (intx)0) == 0) {
> -        if (store_context(ucVoid)) {
> -          g_assertion_context = &g_stored_assertion_context;
> -        }
> +        store_context(ucVoid);
> +        g_assertion_context = &g_stored_assertion_context;
>        }
>      }
>      return true;
>
>
> The store never can go wrong, so returning an error is not needed.
>
> As a background, the function is part of a change which provides the
> abilities to see register values in hs-err files in assert/guarantee
> situations, see https://bugs.openjdk.java.net/browse/JDK-8191101 .
>
> Thanks, Thomas
>
>
> On Tue, Jun 19, 2018 at 3:29 PM, Martin Buchholz <martinrb at google.com>
> wrote:
> > if the check of memcpy's return value goes away, then store_context
> always
> > returns true, which seems fishy.   A code owner should decide what should
> > really happen here.
> >
> > On Mon, Jun 18, 2018 at 11:08 PM, David Holmes <david.holmes at oracle.com>
> > wrote:
> >>
> >> On 19/06/2018 1:00 PM, Martin Buchholz wrote:
> >>
> >> AFAICS the memcpy can't fail and the dest is not NULL so the "if" should
> >> just disappear - no?
> >>
> >> David
> >>
> >>
> >
>


More information about the hotspot-runtime-dev mailing list