RFR: 8205199: more Linux clang compile failures

[David Holmes]

Looks fine Martin!

Thanks,
David

On 20/06/2018 6:07 AM, Martin Buchholz wrote:
> 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 
> <mailto: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
>     <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 <mailto: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 <mailto: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
>      >>
>      >>
>      >
> 
>