RFR: 8205199: more Linux clang compile failures

[Thomas Stüfe]

Thanks. Change looks good to me.

..Thomas

On Tue 19. Jun 2018 at 22:07, Martin Buchholz <martinrb at google.com> 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>
> 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
>> >>
>> >>
>> >
>>
>
>