Re: bin/94767: [patch] rcorder(8) dumps core when does not use a proper RCng script (dansguardian)

I tested your test script and found out the way to avoid core dump in 
rcorder.
I didn't quite get the #if 0 lines that you provide in patch.
Correct patch might be in following way:

-------------------------------------------------
--- rcorder.c.orig      Tue Jan 17 16:01:00 2006
+++ rcorder.c   Mon Mar 27 16:32:18 2006
_at__at_ -757,8 +757,11 _at__at_
                r_tmp = r;
                satisfy_req(r, fnode->filename);
                r = r->next;
-               if (was_set == 0)
+               if (was_set == 0) {
+#if 0
                        free(r_tmp);
+#endif
+               }
        }
        fnode->req_list = NULL;

_at__at_ -796,10 +799,14 _at__at_
                fnode->last->next = fnode->next;
        }

-       DPRINTF((stderr, "nuking %s\n", fnode->filename));
        if (was_set == 0) {
-               free(fnode->filename);
-               free(fnode);
+               DPRINTF((stderr, "nuking %s\n", fnode->filename));
+#if 0
+               free(fnode->filename);
+               free(fnode);
+#else
+               DPRINTF((stderr, "but skipping it for now\n"));
+#endif
        }
 }
-------------------------------------------------

But, I looked at the Dragonfly rcorder.c code 
(http://gobsd.com/code/dragonfly/sbin/rcorder/rcorder.c)
and they just disabled the lines which frees the memory. If we follow 
Dragonfly code
the patch might look like the following (I just commented out those lines):

-------------------------------------------------
--- rcorder.c.orig      Tue Jan 17 16:01:00 2006
+++ rcorder.c   Mon Mar 27 16:41:49 2006
_at__at_ -757,8 +757,8 _at__at_
                r_tmp = r;
                satisfy_req(r, fnode->filename);
                r = r->next;
-               if (was_set == 0)
-                       free(r_tmp);
+/*             if (was_set == 0)*/
+/*                     free(r_tmp);*/
        }
        fnode->req_list = NULL;

_at__at_ -797,10 +797,10 _at__at_
        }

        DPRINTF((stderr, "nuking %s\n", fnode->filename));
-       if (was_set == 0) {
-               free(fnode->filename);
-               free(fnode);
-       }
+/*     if (was_set == 0) {*/
+/*             free(fnode->filename);*/
+/*             free(fnode);*/
+/*     }*/
 }

 void
-------------------------------------------------
So I'm bit confused here which type of patch we should apply.
I guess we should follow Dragonfly. Maybe I' wrong.
Can somebody enlighten me here?

thanks,

Ganbold


Adrian Steinmann wrote:
>
> I've run into this problem on -current too and have created three
> test cases which dump core. With the mentioned patch two of the
> cases are fixed, but one remains: It seems the "don't nuke unless
> last node" patch works when no third "stub dependancy" interferes.
>
> On 6.1-PRERELEASE all three test cases pass.
>
> Adrian
>
> Details:
>
> #!/bin/sh
>
> try()
> {
>     [ $# -lt 2 ] && return
>     echo "=============== TEST rcorder $_at_"
>     files=$_at_
>     cmd="rcorder $files"
>     for f in $files
>     do
>         echo === cat $f:
>         cat $f
>     done
>     rm -f rcorder.core
>     echo === $cmd:
>     eval $cmd
>     [ -e rcorder.core ] && echo "DUMPED CORE - ^C to stop" && read a
>     rm -f rcorder.core $_at_
>     echo
> }
>
> { echo "# PROVIDE: A"; echo "# REQUIRE: B"; } >A
> { echo "# PROVIDE: B"; echo "# REQUIRE: A"; } >B
> try A B
>
> { echo "# PROVIDE: A"; echo "# REQUIRE: B C"; } >A
> { echo "# PROVIDE: B"; echo "# REQUIRE: A C"; } >B
> { echo "# PROVIDE: C"; } > C
> try A B C
>
> { echo "# PROVIDE: A"; echo "# REQUIRE: B"; } >A
> { echo "# PROVIDE: B"; echo "# REQUIRE: C"; } >B
> { echo "# PROVIDE: C"; echo "# REQUIRE: A"; } >C
> try A B C
>
> exit 0
>
>
> Index: rcorder.c
> ===================================================================
> RCS file: /usr/cvs/src/sbin/rcorder/rcorder.c,v
> retrieving revision 1.2
> diff -u -r1.2 rcorder.c
> --- rcorder.c   17 Jan 2006 08:01:00 -0000      1.2
> +++ rcorder.c   25 Mar 2006 16:40:33 -0000
> _at__at_ -51,8 +51,9 _at__at_
>  #include "sprite.h"
>  #include "hash.h"
>
> +#define DEBUG 1
>  #ifdef DEBUG
> -int debug = 0;
> +int debug = 1;
>  # define       DPRINTF(args) if (debug) { fflush(stdout); fprintf 
> args; }
>  #else
>  # define       DPRINTF(args)
>
> here, all three test cases dump core, apparently the stack is being hit
> in the first and third case:
>
> =============== TEST1 rcorder A B
> ..
> generate on ZZZZZZZZZZZZZZZZZZZZ
> do_file on ZZZZZZZZZZZZZZZZZZZZ.
> Segmentation fault (core dumped)
> DUMPED CORE - ^C to stop
>
> Core was generated by `rcorder'.
> Program terminated with signal 11, Segmentation fault.
> #0  0x080493ac in satisfy_req (rnode=0x5a5a5a5a,
>     filename=0xbfbfe7b0 "?㿿\003\001") at 
> /usr/src/sbin/rcorder/rcorder.c:655
> 655             entry = rnode->entry;
> (gdb) where
> #0  0x080493ac in satisfy_req (rnode=0x5a5a5a5a,
>     filename=0xbfbfe7b0 "?㿿\003\001") at 
> /usr/src/sbin/rcorder/rcorder.c:655
> #1  0x08048fa3 in do_file (fnode=0xa000110)
>     at /usr/src/sbin/rcorder/rcorder.c:759
> #2  0x0804917b in generate_ordering () at 
> /usr/src/sbin/rcorder/rcorder.c:827
> #3  0x08049369 in main (argc=2, argv=0x1)
>     at /usr/src/sbin/rcorder/rcorder.c:193
>
> =============== TEST2 rcorder A B C
> Program terminated with signal 11, Segmentation fault.
> #0  satisfy_req (rnode=0xa000374, filename=0xa0001a0 "B")
>     at /usr/src/sbin/rcorder/rcorder.c:666
> 666             if (head->next == NULL)
> (gdb) where
> #0  satisfy_req (rnode=0xa000374, filename=0xa0001a0 "B")
>     at /usr/src/sbin/rcorder/rcorder.c:666
> #1  0x08048fa3 in do_file (fnode=0xa0003e0)
>     at /usr/src/sbin/rcorder/rcorder.c:759
> #2  0x080493d7 in satisfy_req (rnode=0xa000374, filename=0xa0001a0 "B")
>     at /usr/src/sbin/rcorder/rcorder.c:687
> #3  0x08048fa3 in do_file (fnode=0xa000110)
>     at /usr/src/sbin/rcorder/rcorder.c:759
> #4  0x080493d7 in satisfy_req (rnode=0xa000374, filename=0xa0001a0 "B")
>     at /usr/src/sbin/rcorder/rcorder.c:687
> #5  0x08048fa3 in do_file (fnode=0xa0003e0)
>     at /usr/src/sbin/rcorder/rcorder.c:759
> #6  0x0804917b in generate_ordering () at 
> /usr/src/sbin/rcorder/rcorder.c:827
> #7  0x08049369 in main (argc=3, argv=0x1)
>     at /usr/src/sbin/rcorder/rcorder.c:193
>
> =============== TEST3 rcorder A B C
> Program terminated with signal 11, Segmentation fault.
> #0  0x080493ac in satisfy_req (rnode=0x5a5a5a5a,
>     filename=0xbfbfe7b0 "?㿿???\n") at 
> /usr/src/sbin/rcorder/rcorder.c:655
> 655             entry = rnode->entry;
> (gdb) where
> #0  0x080493ac in satisfy_req (rnode=0x5a5a5a5a,
>     filename=0xbfbfe7b0 "?㿿???\n") at 
> /usr/src/sbin/rcorder/rcorder.c:655
> #1  0x08048fa3 in do_file (fnode=0xa000360)
>     at /usr/src/sbin/rcorder/rcorder.c:759
> #2  0x0804917b in generate_ordering () at 
> /usr/src/sbin/rcorder/rcorder.c:827
> #3  0x08049369 in main (argc=3, argv=0x1)
>     at /usr/src/sbin/rcorder/rcorder.c:193
>
>
> with the patch proposed earlier (skip nuking)
>
> Index: rcorder.c
> ===================================================================
> RCS file: /usr/cvs/src/sbin/rcorder/rcorder.c,v
> retrieving revision 1.2
> diff -u -r1.2 rcorder.c
> --- rcorder.c   17 Jan 2006 08:01:00 -0000      1.2
> +++ rcorder.c   25 Mar 2006 16:45:38 -0000
> _at__at_ -51,8 +51,9 _at__at_
>  #include "sprite.h"
>  #include "hash.h"
>
> +#define DEBUG 1
>  #ifdef DEBUG
> -int debug = 0;
> +int debug = 1;
>  # define       DPRINTF(args) if (debug) { fflush(stdout); fprintf 
> args; }
>  #else
>  # define       DPRINTF(args)
> _at__at_ -796,10 +797,14 _at__at_
>                 fnode->last->next = fnode->next;
>         }
>
> -       DPRINTF((stderr, "nuking %s\n", fnode->filename));
>         if (was_set == 0) {
> +               DPRINTF((stderr, "nuking %s\n", fnode->filename));
> +#if 0
>                 free(fnode->filename);
>                 free(fnode);
> +#else
> +               DPRINTF((stderr, "but skipping it for now\n"));
> +#endif
>         }
>  }
>
> only TEST2 dumps core:
>
> =============== TEST2 rcorder A B C
> === cat A:
> # PROVIDE: A
> # REQUIRE: B C
> === cat B:
> # PROVIDE: B
> # REQUIRE: A C
> === cat C:
> # PROVIDE: C
> === rcorder A B C:
> parse_args
> initialize
> crunch_all_files
> generate on C
> do_file on C.
> next do: C
> nuking C
> but skipping it for now
> generate on B
> do_file on B.
> do_file on A.
> do_file on B.
> rcorder: Circular dependency on file `B'.
> Segmentation fault (core dumped)
>
> Program terminated with signal 11, Segmentation fault.
> #0  0x080493d0 in satisfy_req ()
> (gdb) where
> #0  0x080493d0 in satisfy_req ()
> #1  0x08048fa3 in do_file ()
> #2  0x080493ef in satisfy_req ()
> #3  0x08048fa3 in do_file ()
> #4  0x080493ef in satisfy_req ()
> #5  0x08048fa3 in do_file ()
> #6  0x08049193 in generate_ordering ()
> #7  0x08049381 in main ()