tags 123272 - patch
thanks
Aurélien GÉRÔME <[EMAIL PROTECTED]> writes:
>> * nsub is an optional argument; its absence is not an error.
>
> It leads to SIGSEGV, so it seems to me it is indeed.
The documentation is clear:
crashme.txt:45: [NSUB] The [NSUB] is optional, the number of vfork sub-
crashme.txt:46: processes running all at once. [...]
Removing features is not the way to fix bugs.
>> * The if branch you are patching doesn't actually use nsub; only the
>> first three arguments are passed to old_main().
>
> I do not see that when I read the thing with indent as my helper.
Videlicet:
crashme.c:392: else if ((argc == 6) && ((strlen(argv[4]) == 0) ||
crashme.c:393: (strcmp(argv[4],".") == 0)))
crashme.c:394: {verbose_level = atol(argv[5]);
crashme.c:395: old_main(4,argv);}
The first condition on line 392 checks that there are five arguments
(plus the program name); this guarantees that argv[4] is non-null, and
the other two conditions will not produce a segfault. Line 394 stores
the fifth argument (VERBOSE) in verbose_level. Line 395 calls
old_main() with an unchanged argv[].
As you can see below, old_main() neither accesses argv[4] nor passes
argv[] to other functions:
crashme.c:434:void old_main(argc,argv)
crashme.c:435: int argc;
crashme.c:436: char **argv;
crashme.c:437:{char *ptr;
crashme.c:438: copyright_note(3);
crashme.c:439: nbytes = atol(argv[1]);
crashme.c:440: if (ptr = strchr(argv[1],'.'))
crashme.c:441: incptr = atol(&ptr[1]);
crashme.c:442: if (argv[1][0] == '+') malloc_flag = 1;
crashme.c:443: nseed = atol(argv[2]);
crashme.c:444: ntrys = atol(argv[3]);
crashme.c:445: sprintf(notes,"crashme %s%ld.%d %ld %ld",
crashme.c:446: (malloc_flag == 0) ? "" :
"+",nbytes,incptr,nseed,ntrys);
crashme.c:447: note(3);
crashme.c:448: record_note();
crashme.c:449: if (malloc_flag == 0)
crashme.c:450: {the_data = bad_malloc((nbytes < 0) ? -nbytes : nbytes);
crashme.c:451: badboy = castaway(the_data);
crashme.c:452: sprintf(notes,"Badboy at %d. 0x%X",badboy,badboy);
crashme.c:453: note(3);}
crashme.c:454: srand(nseed);
crashme.c:455:#ifdef WIN32
crashme.c:456: SetErrorMode(SEM_FAILCRITICALERRORS |
crashme.c:457: SEM_NOGPFAULTERRORBOX |
crashme.c:458: SEM_NOOPENFILEERRORBOX);
crashme.c:459:#endif
crashme.c:460: badboy_loop();}
This leads me to conclude that argv[4] is not the cause of the crash.
>> * From a cursory gdb session, the reason crashme segfaults is that it
>> executes illegal instructions, not in the options parsing.
>
> So why does it not segfault anymore with my work?
$ ./patched-crashme +8192.16 478 100
[...]
try 38, offset 608
Segmentation fault
> While you seem perfectly fine saying what I did is stupid,
I did not say it was stupid, I said it was incorrect, and why. Do you
see this as a personal insult?
> I would be rather interested that you propose a better solution for
> a bug opened more than 4 years ago and which nobody cared to fix.
I couldn't find it at first glance and gave up; the fact that no one
cared to in 4 years is telling me something.
Cheers,
Matej
