Re: [PATCH 1/2] Add constructor and destructor to thread_info

[Pedro Alves <palves at redhat dot com>]

On 03/28/2017 04:40 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>>>>  - struct target_waitstatus pending_follow;
>>>>  + struct target_waitstatus pending_follow {TARGET_WAITKIND_SPURIOUS};
>>>>
>>>
>>> This will only initialize the first member of union
>>> target_waitstatus.value to zero, so some bits of
>>> target_waitstatus.value.related_pid is not initialized.
>>
>> Ah, blah, unions.  Well, the result is that the remaining fields of
>> the struct end up list->value->zero-initialized.  Zero-initialization
>> for unions means the first member is zero-initialized, and padding
>> is initialized to zero.  So I think the end result is the
>> same anyway.
> 
> It is different from
> "memset (&this->pending_follow, 0, sizeof (this->pending_follow))".  The
> first member "integer" is zero-initialized, but it is not the largest
> member, so part of "related_pid" is not zero-initialized.
> 
>   |<------------------ union----------------->|
>   |<------- related_pid ------->|<- padding ->|
>   |<- integer ->|
>   |      0      |<uninitialized>|     0       |
> 

I'm not so sure about that.  That is not my interpretation,
though the standard isn't crystal clear here.  The definition
of "padding" is left vague.  It doesn't look like that's the
interpretation of either G++ nor Clang though.  G++ always zeroes
the whole object, while Clang initializes only the first member,
leaving everything else uninitialized, which I found surprising,
since no padding _at all_ of any kind is initialized then.

I've sent an inquiry to std-discussion at isocpp.org list earlier
today, though I haven't gotten an answer yet.  See:

  https://groups.google.com/a/isocpp.org/forum/#!topic/std-discussion/ljaMNnkahHA

Though given Clang's behavior, it's probably not a good idea to assume
everything's zeroed out.

target_waitstatus would be a natural fit for something like
C++17's std::variant.  Particularly, the lifetime of value.execd_pathname is
not well defined...  Let's go with what you had, and revisit if/when we
change/fix target_waitstatus.

Thanks,
Pedro Alves