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

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

>  >  "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