Date: Sun, 2 Jul 2023 06:18:35 -0400 From: Ka Ho Ng <khng300@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Ka Ho Ng <khng@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 005aa1743b42 - main - modules: bzero the modspecific_t Message-ID: <CANnchUZtmWii1prVROt9NLMfCTfi4POzw21Bg-cmnP3gmMGqww@mail.gmail.com> In-Reply-To: <CANnchUYAMtSDgUOVteLQ9437=p-0WL2ie4oYg94qhjSEkmxk_Q@mail.gmail.com> References: <202307012259.361MxM4i017090@gitrepo.freebsd.org> <ZKCzDPfJM2gGrhYT@kib.kiev.ua> <CANnchUYAMtSDgUOVteLQ9437=p-0WL2ie4oYg94qhjSEkmxk_Q@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] 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 [-- Attachment #2 --] <div dir="ltr"><div dir="ltr">On Sun, Jul 2, 2023 at 6:03 AM Ka Ho Ng <<a href="mailto:khng300@gmail.com">khng300@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Sat, Jul 1, 2023 at 7:13 PM Konstantin Belousov <<a href="mailto:kostikbel@gmail.com" target="_blank">kostikbel@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sat, Jul 01, 2023 at 10:59:22PM +0000, Ka Ho Ng wrote:<br> > The branch main has been updated by khng:<br> > <br> > URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=005aa1743b42b52fbd49b9d5ec44816902b6ee9f" rel="noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=005aa1743b42b52fbd49b9d5ec44816902b6ee9f</a><br>; > <br> > commit 005aa1743b42b52fbd49b9d5ec44816902b6ee9f<br> > Author: Ka Ho Ng <khng@FreeBSD.org><br> > AuthorDate: 2023-07-01 19:41:53 +0000<br> > Commit: Ka Ho Ng <khng@FreeBSD.org><br> > CommitDate: 2023-07-01 22:58:46 +0000<br> > <br> > modules: bzero the modspecific_t<br> > <br> > Per <a href="https://reviews.llvm.org/D68115" rel="noreferrer" target="_blank">https://reviews.llvm.org/D68115</a>, only the first field is<br> > zero-initialized, meanwhile other fields are undef.<br> This is not true.<br> <br> > <br> > The pattern can be observed on clang as well, that when<br> > -ftrivial-auto-var-init=pattern is specified 0xaa is filled for<br> > non-active fields, otherwise they are zero-initialized.<br> What are non-active fields?<br> All fields with implicit initializers<br> "shall be initialized implicitly the same as<br> objects that have static storage duration."<br> <br> I do not think this is required for padding.<br></blockquote><div>In that case, modspecific_t ms did become 0xaaaaaaaa00000000 on amd64 if -ftrivial-auto-var-init=pattern was specified.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br> > Technically both are acceptable when using clang. However it<br> > would be good to simply bzero the modspecific_t in such case to<br> > be strict to the standard.<br> > <br> > MFC with: 2cab2d43b83b<br> > MFC after: 1 day<br> Min MFC period is 3 days.<br> <br> > Sponsored by: Juniper Networks, Inc.<br> > Reviewed by: delphij<br> > Differential Revision: <a href="https://reviews.freebsd.org/D40830" rel="noreferrer" target="_blank">https://reviews.freebsd.org/D40830</a><br>; > ---<br> > sys/kern/kern_syscalls.c | 3 ++-<br> > 1 file changed, 2 insertions(+), 1 deletion(-)<br> > <br> > diff --git a/sys/kern/kern_syscalls.c b/sys/kern/kern_syscalls.c<br> > index 234e51cfd280..0b51edf5e985 100644<br> > --- a/sys/kern/kern_syscalls.c<br> > +++ b/sys/kern/kern_syscalls.c<br> > @@ -173,9 +173,10 @@ kern_syscall_module_handler(struct sysent *sysents, struct module *mod,<br> > int what, void *arg)<br> > {<br> > struct syscall_module_data *data = arg;<br> > - modspecific_t ms = { 0 };<br> > + modspecific_t ms;<br> > int error;<br> > <br> > + bzero(&ms, sizeof(ms));<br> > switch (what) {<br> > case MOD_LOAD:<br> > error = kern_syscall_register(sysents, data->offset,</blockquote><div><br></div><div>Ka Ho</div></div></div></blockquote><div>Since I missed the reply-all button just now, I resend this email.</div><div><br></div><div>Reading on N2716 one of the footnote stated:<br>"Note that aggregate type does not include union type because an object with union type can only contain one member at a time"<br><br></div><div>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.</div><div><br></div><div>Ka Ho <br></div></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANnchUZtmWii1prVROt9NLMfCTfi4POzw21Bg-cmnP3gmMGqww>