Re: bin/40036: rc.d/network doesn't check IPv6 properly with newifconfig(8)

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

The following reply was made to PR bin/40036; it has been noted by GNATS.

From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
To: dholland-bugs%netbsd.org@localhost
Cc: gnats-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, 
netbsd-bugs%netbsd.org@localhost,
        tsutsui%ceres.dti.ne.jp@localhost
Subject: Re: bin/40036: rc.d/network doesn't check IPv6 properly with 
newifconfig(8)
Date: Fri, 28 Nov 2008 19:39:08 +0900

 >  >  "cc -O1 -fno-loop-optimize" fixes the problem, but
 >  >  the attached changes also fixes it.
 >  >  
 >  >  I'm not sure if it's a gcc optimization bug (on m68000 or m68k+softfloat)
 >  >  or a source bug, but is it okay to commit this diff for workaround?
 > 
 > That would be a gcc bug.
 
 It turns out that the similar bug could occur even on i386
 if -O1 is specified (but not with -O2):
 ---
 % uname -mrs
 NetBSD 5.0_BETA i386
 % gcc --version
 gcc (GCC) 4.1.3 20080704 prerelease (NetBSD nb1 20080202)
 Copyright (C) 2006 Free Software Foundation, Inc.
 This is free software; see the source for copying conditions.  There is NO
 warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 
 % cat test.c
 #include <inttypes.h>
 #include <stdio.h>
 
 struct kwinst {
        int                     k_type;
        const char              *k_word;
        union kwval {
                int64_t         u_sint;
                char            *u_str;
        } k_u;
 #define k_int   k_u.u_sint
 };
 
 struct kwinst fkw[10];
 int num[] = { 11, 22, 33, 44 };
 char *name[] = {"aaaaaa","bbbbbb","cccccc","dddddd"};
 
 main()
 {
        int a, i;
        struct kwinst kw = {.k_type = 1};
 
        for (a = 0; a < 4; a++) {
                kw.k_int = num[a];
                kw.k_word = name[a];
                for (i = 0; i < 10; i++) {
                        if (fkw[i].k_word == NULL) {
                                fkw[i] = kw;
                                break;
                        }
                }
                printf("a=%d: kw.k_word=%s kw.k_int=%d, "
                    "fkw[%d].k_word=%s fkw[%d].k_int=%d\n", a,
                    kw.k_word, (int)kw.k_int,
                    i, fkw[i].k_word, i, (int)fkw[i].k_int);
        }
 }
 
 % cc -O test.c
 % ./a.out
 a=0: kw.k_word=aaaaaa kw.k_int=11, fkw[0].k_word=(null) fkw[0].k_int=11
 a=1: kw.k_word=bbbbbb kw.k_int=22, fkw[0].k_word=aaaaaa fkw[0].k_int=22
 a=2: kw.k_word=cccccc kw.k_int=33, fkw[1].k_word=bbbbbb fkw[1].k_int=33
 a=3: kw.k_word=dddddd kw.k_int=44, fkw[2].k_word=cccccc fkw[2].k_int=44
 % cc -O -fno-loop-optimize test.c
 % ./a.out
 a=0: kw.k_word=aaaaaa kw.k_int=11, fkw[0].k_word=aaaaaa fkw[0].k_int=11
 a=1: kw.k_word=bbbbbb kw.k_int=22, fkw[1].k_word=bbbbbb fkw[1].k_int=22
 a=2: kw.k_word=cccccc kw.k_int=33, fkw[2].k_word=cccccc fkw[2].k_int=33
 a=3: kw.k_word=dddddd kw.k_int=44, fkw[3].k_word=dddddd fkw[3].k_int=44
 % cc -O2 test.c
 % ./a.out
 a=0: kw.k_word=aaaaaa kw.k_int=11, fkw[0].k_word=aaaaaa fkw[0].k_int=11
 a=1: kw.k_word=bbbbbb kw.k_int=22, fkw[1].k_word=bbbbbb fkw[1].k_int=22
 a=2: kw.k_word=cccccc kw.k_int=33, fkw[2].k_word=cccccc fkw[2].k_int=33
 a=3: kw.k_word=dddddd kw.k_int=44, fkw[3].k_word=dddddd fkw[3].k_int=44
 % 
 ---
 
 >  >  -        struct kwinst kw = {.k_type = KW_T_INT};
 > 
 > However, this initializes all the fields of struct kwinst (there are
 > quite a few) and the new code without kw does not. It probably doesn't
 > matter, as familykw[] is file-static,
 
 I assumed it in the patch.
 
 > but maybe it's better to be
 > cautious.
 > 
 > (Does making kw volatile cause the problem to go away?)
 
 Yes, but it looks weird for workaround.
 
 Maybe we have the following options:
 
 1) add COPTS.ifconfig.c+= -fno-loop-optimize in src/sbin/ifconfig/Makefile
    .if ${MACHINE_ARCH} == "m68000"
 2) add COPTS.ifconfig.c+= -fno-loop-optimize in src/sbin/ifconfig/Makefile
    by default
 3) use DBG?= -O1 -fno-loop-optimize in src/share/mk/sys.mk for m68000
 4) put workaround changes into ifconfig.c
 5) fix gcc (best but probably unlikely)
 
 Opinions?
 
 ---
 Izumi Tsutsui