Re: git: 005aa1743b42 - main - modules: bzero the modspecific_t

On Sun, Jul 2, 2023 at 6:03 AM Ka Ho Ng <khng300@gmail.com> wrote:

> On Sat, Jul 1, 2023 at 7:13 PM Konstantin Belousov <kostikbel@gmail.com>
> wrote:
>
>> On Sat, Jul 01, 2023 at 10:59:22PM +0000, Ka Ho Ng wrote:
>> > The branch main has been updated by khng:
>> >
>> > URL:
>> https://cgit.FreeBSD.org/src/commit/?id=005aa1743b42b52fbd49b9d5ec44816902b6ee9f
>> >
>> > commit 005aa1743b42b52fbd49b9d5ec44816902b6ee9f
>> > Author:     Ka Ho Ng <khng@FreeBSD.org>
>> > AuthorDate: 2023-07-01 19:41:53 +0000
>> > Commit:     Ka Ho Ng <khng@FreeBSD.org>
>> > CommitDate: 2023-07-01 22:58:46 +0000
>> >
>> >     modules: bzero the modspecific_t
>> >
>> >     Per https://reviews.llvm.org/D68115, only the first field is
>> >     zero-initialized, meanwhile other fields are undef.
>> This is not true.
>>
>> >
>> >     The pattern can be observed on clang as well, that when
>> >     -ftrivial-auto-var-init=pattern is specified 0xaa is filled for
>> >     non-active fields, otherwise they are zero-initialized.
>> What are non-active fields?
>> All fields with implicit initializers
>> "shall be initialized implicitly the same as
>> objects that have static storage duration."
>>
>> I do not think this is required for padding.
>>
> In that case, modspecific_t ms did become 0xaaaaaaaa00000000 on amd64 if
> -ftrivial-auto-var-init=pattern was specified.
>
>
>>
>> >     Technically both are acceptable when using clang. However it
>> >     would be good to simply bzero the modspecific_t in such case to
>> >     be strict to the standard.
>> >
>> >     MFC with:       2cab2d43b83b
>> >     MFC after:      1 day
>> Min MFC period is 3 days.
>>
>> >     Sponsored by:   Juniper Networks, Inc.
>> >     Reviewed by:    delphij
>> >     Differential Revision:  https://reviews.freebsd.org/D40830
>> > ---
>> >  sys/kern/kern_syscalls.c | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/sys/kern/kern_syscalls.c b/sys/kern/kern_syscalls.c
>> > index 234e51cfd280..0b51edf5e985 100644
>> > --- a/sys/kern/kern_syscalls.c
>> > +++ b/sys/kern/kern_syscalls.c
>> > @@ -173,9 +173,10 @@ kern_syscall_module_handler(struct sysent
>> *sysents, struct module *mod,
>> >      int what, void *arg)
>> >  {
>> >       struct syscall_module_data *data = arg;
>> > -     modspecific_t ms = { 0 };
>> > +     modspecific_t ms;
>> >       int error;
>> >
>> > +     bzero(&ms, sizeof(ms));
>> >       switch (what) {
>> >       case MOD_LOAD:
>> >               error = kern_syscall_register(sysents, data->offset,
>
>
> Ka Ho
>
Since I missed the reply-all button just now, I resend this email.

Reading on N2716 one of the footnote stated:
"Note that aggregate type does not include union type because an object
with union type can only contain one member at a time"

And below at 6.7.9.21 only aggregate was mentioned but not union, which
matched what I observed with an `cc -ftrivial-auto-var-init=pattern`
invocation.

Ka Ho