Re: svn commit: r330602 - head/sys/compat/cloudabi
On Wed, 7 Mar 2018, Benjamin Kaduk wrote: On Wed, Mar 7, 2018 at 9:44 AM, Ed Mastewrote: On 7 March 2018 at 09:47, Eitan Adler wrote: ... Log: sys/cloudabi: Avoid relying on GNU specific extensions An empty initializer list is not technically valid C grammar. MFC After:1 week - cloudabi_fdstat_t fsb = {}; + cloudabi_fdstat_t fsb = {0}; In practice it appears initializing via { 0 } also zeros any padding in the struct, but I do not believe it's required by the C standard. Perhaps a language lawyer can weigh in? Commenting on this commit just because it's highlighted by this change; I do not believe there's a difference between the GNU extension { } and { 0 } here. It is also a style bug to initialize variables in declarations. It is interesting that this style bug gives other bugs that are more serious than when the style rule was new: - locking is often needed before complicated initializations. It would be an even larger style bug to write the lock acquisition in initializers, and C doesn't have finalizers so it is impossible to obfuscate the lock release by writing it in finalizers - this problem of initializing padding. Auto initializers also used to be good for pessimizations. Use them instead of static initializers for constant values. This asks the compiler to initialize them on every entry to the function. It was a typical implementation to keep the values in an unnamed static object and copy this to the stack at runtime. Now compilers are more likely to optimize away the copying, so the pessimization doesn't work so well. Copying from a static object tends to give zero padding, but optimizated variants should only give zero padding if that is optimal. The C spec says that if an incomplete initializer is given, then all other fields of the structure are initialized to zero. The state of padding is unspecified, whether a complete or incomplete initializer is given. This seems to apply to static objects too. So initializing dynamic objects by copying them from static objects is insecure even if you copy using memcpy() (copying using struct assignment might skip the padding so shouldn't be used). A malicious compiler could initialize the padding with security-related info. non-malicious compiler might initialize the padding with stack garbage that happens to be security-related. Bruce ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r330602 - head/sys/compat/cloudabi
On Wed, Mar 7, 2018 at 8:44 AM, Ed Mastewrote: > On 7 March 2018 at 09:47, Eitan Adler wrote: > > Author: eadler > > Date: Wed Mar 7 14:47:43 2018 > > New Revision: 330602 > > URL: https://svnweb.freebsd.org/changeset/base/330602 > > > > Log: > > sys/cloudabi: Avoid relying on GNU specific extensions > > > > An empty initializer list is not technically valid C grammar. > > > > MFC After:1 week > > > > - cloudabi_fdstat_t fsb = {}; > > + cloudabi_fdstat_t fsb = {0}; > > In practice it appears initializing via { 0 } also zeros any padding > in the struct, but I do not believe it's required by the C standard. > Perhaps a language lawyer can weigh in? > All the standard says are that all named are initialized to 0 (which has the usual meaning for the integer 0 for pointers) unless otherwise stated: 6.7.8 para 9: "Except where explicitly stated otherwise, for the purposes of this subclause unnamed members of objects of structure and union type do not participate in initialization. Unnamed members of structure objects have indeterminate value even after initialization." 6.7.8 para 21: "If there are fewer initializers in a brace-enclosed list than there are elements or members of an aggregate, or fewer characters in a string literal used to initialize an array of known size than there are elements in the array, the remainder of the aggregate shall be initialized implicitly the same as objects that have static storage duration." The aggregate here, I believe refers to the elements, not the padded structure, and the padding is definitely an unnamed part of the structure, which makes it unspecified. However 6.7.8 is quite long and detailed, and I'm not 100% sure I have totally grokked all its subtle implications. Commenting on this commit just because it's highlighted by this > change; I do not believe there's a difference between the GNU > extension { } and { 0 } here. > I believe that's correct as well. Warner ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r330602 - head/sys/compat/cloudabi
On Wed, Mar 7, 2018 at 9:44 AM, Ed Mastewrote: > On 7 March 2018 at 09:47, Eitan Adler wrote: > > Author: eadler > > Date: Wed Mar 7 14:47:43 2018 > > New Revision: 330602 > > URL: https://svnweb.freebsd.org/changeset/base/330602 > > > > Log: > > sys/cloudabi: Avoid relying on GNU specific extensions > > > > An empty initializer list is not technically valid C grammar. > > > > MFC After:1 week > > > > - cloudabi_fdstat_t fsb = {}; > > + cloudabi_fdstat_t fsb = {0}; > > In practice it appears initializing via { 0 } also zeros any padding > in the struct, but I do not believe it's required by the C standard. > Perhaps a language lawyer can weigh in? > > Commenting on this commit just because it's highlighted by this > change; I do not believe there's a difference between the GNU > extension { } and { 0 } here. > > The C spec says that if an incomplete initializer is given, then all other fields of the structure are initialized to zero. The state of padding is unspecified, whether a complete or incomplete initializer is given. The "issue" being "fixed" here is that the formal C grammar does not admit a totally empty initializer, so at least one element of the initializer needs to be given in order to satisfy the grammar. This is pretty silly, and the extension to allow a totally empty initializer a quite natural one to make. -Ben ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r330602 - head/sys/compat/cloudabi
On 7 March 2018 at 09:47, Eitan Adlerwrote: > Author: eadler > Date: Wed Mar 7 14:47:43 2018 > New Revision: 330602 > URL: https://svnweb.freebsd.org/changeset/base/330602 > > Log: > sys/cloudabi: Avoid relying on GNU specific extensions > > An empty initializer list is not technically valid C grammar. > > MFC After:1 week > > - cloudabi_fdstat_t fsb = {}; > + cloudabi_fdstat_t fsb = {0}; In practice it appears initializing via { 0 } also zeros any padding in the struct, but I do not believe it's required by the C standard. Perhaps a language lawyer can weigh in? Commenting on this commit just because it's highlighted by this change; I do not believe there's a difference between the GNU extension { } and { 0 } here. ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"