Bug 243974
| Summary: | diff: generates core dump when --tabsize is used | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | fehmi noyan isi <fnoyanisi> | ||||||
| Component: | bin | Assignee: | Kyle Evans <kevans> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Some People | CC: | emaste, fnoyanisi, kevans, rlwestlund | ||||||
| Priority: | --- | Keywords: | crash | ||||||
| Version: | 12.1-RELEASE | Flags: | kevans:
mfc-stable12+
kevans: mfc-stable11+ kevans: mfc-stable10- |
||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
duplicate of https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241714 The resolution mentioned in the above bug report is to pass the argument following --tabsize after an equal sign, e,g, --tabsize=NUMBER as opposed to --tabsize NUMBER. In diff.c, the argument to tabsize is classified as "optional_argument" whereas it should be "required_argument". We either should; - update the man page and indicate the argument following --tabsize is optional - make the argument to --tabsize required. I am more keen to go with the second option (i.e. making the argument mandatory for the --tabsize). It does not make much sense to use only --tabsize Created attachment 211493 [details]
patch for diff.c
this patch changes the NUMBER argument following --tabsize from optional to required.
POC for the proposed patch fnoyanisi@patch:~ $ diff A B 1,5c1,3 < same1 < differ1 < < < differ3 --- > same1 > differ2 > differ4 fnoyanisi@patch:~ $ diff A B --tabsize 4 Segmentation fault (core dumped) // After the patch fnoyanisi@patch:~ $ diff A B --tabsize 4 1,5c1,3 < same1 < differ1 < < < differ3 --- > same1 > differ2 > differ4 fnoyanisi@patch:~ $ uname -a FreeBSD patch 13.0-CURRENT FreeBSD 13.0-CURRENT #0 r357606: Thu Feb 6 04:40:35 UTC 2020 root@releng1.nyi.freebsd.org:/usr/obj/usr/src/amd64.amd64/sys/GENERIC amd64 fnoyanisi@patch:~ $ // No failing ATF tests root@patch:/usr/tests # kyua test -k /usr/tests/Kyuafile usr.bin/diff usr.bin/diff/diff_test:Bflag -> passed [0.191s] usr.bin/diff/diff_test:b230049 -> passed [0.054s] usr.bin/diff/diff_test:brief_format -> passed [0.180s] usr.bin/diff/diff_test:group_format -> passed [0.064s] usr.bin/diff/diff_test:header -> passed [0.066s] usr.bin/diff/diff_test:header_ns -> passed [0.063s] usr.bin/diff/diff_test:ifdef -> passed [0.060s] usr.bin/diff/diff_test:side_by_side -> passed [0.130s] usr.bin/diff/diff_test:simple -> passed [0.230s] usr.bin/diff/diff_test:unified -> passed [0.097s] usr.bin/diff/netbsd_diff_test:mallocv -> passed [0.069s] usr.bin/diff/netbsd_diff_test:nomallocv -> passed [0.070s] usr.bin/diff/netbsd_diff_test:same -> passed [0.071s] Results file id is usr_tests.20200213-193818-429125 Results saved to /root/.kyua/store/results.usr_tests.20200213-193818-429125.db 13/13 passed (0 failed) root@patch:/usr/tests # There is another diff(1) patch I am working on [1] but I am reluctant to submit more than one patch for the same source file at once. [1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=243975 Created attachment 211617 [details]
git(1) diff against base
I intend to commit the attached diff a little later today, Submitted by: fehmi noyan isi <fnoyanisi yahoo com> (diff.c portion) -- it also consistently shows the argument to tabsize in the man page and adds a test case for diff --tabsize.
A commit references this bug: Author: kevans Date: Thu Feb 13 20:23:56 UTC 2020 New revision: 357875 URL: https://svnweb.freebsd.org/changeset/base/357875 Log: diff: fix segfault with --tabsize and no/malformed argument --tabsize was previously listed as optional_argument, but didn't account for the optionality of it in the argument handling. This is irrelevant -- the manpage doesn't indicate that the argument is optional, and indeed there's no clear interpretation of omitting the argument because there's no other side effect of --tabsize. The "malformed" argument part of the header on this message is simply referring to usage like this: % diff --tabsize 4 A B With an optional_argument, the argument must be attached to the parameter directly (e.g. --tabsize=4), so the argument is effectively NULL with the above invocation as if no argument had been passed. PR: 243974 Submitted by: fehmi noyan isi <fnoyanisi yahoo com> (diff.c portion) MFC after: 3 days Changes: head/usr.bin/diff/diff.1 head/usr.bin/diff/diff.c head/usr.bin/diff/tests/diff_test.sh ^Triage: Assign to committer that resolved. Pending MFC('s).
*** Bug 241714 has been marked as a duplicate of this bug. *** A commit references this bug: Author: kevans Date: Mon Apr 27 22:50:10 UTC 2020 New revision: 360407 URL: https://svnweb.freebsd.org/changeset/base/360407 Log: MFC r357875: diff: fix segfault with --tabsize and no/malformed argument --tabsize was previously listed as optional_argument, but didn't account for the optionality of it in the argument handling. This is irrelevant -- the manpage doesn't indicate that the argument is optional, and indeed there's no clear interpretation of omitting the argument because there's no other side effect of --tabsize. The "malformed" argument part of the header on this message is simply referring to usage like this: % diff --tabsize 4 A B With an optional_argument, the argument must be attached to the parameter directly (e.g. --tabsize=4), so the argument is effectively NULL with the above invocation as if no argument had been passed. PR: 243974 Changes: _U stable/11/ stable/11/usr.bin/diff/diff.1 stable/11/usr.bin/diff/diff.c stable/11/usr.bin/diff/tests/diff_test.sh _U stable/12/ stable/12/usr.bin/diff/diff.1 stable/12/usr.bin/diff/diff.c stable/12/usr.bin/diff/tests/diff_test.sh |
diff(1) causes SIGSEGV when --tabsize is used. Problem is present on CURRENT as well fnoyanisi@beastie:~ % diff A B 1,5c1,3 < same1 < differ1 < < < differ3 --- > same1 > differ2 > differ4 fnoyanisi@beastie:~ % diff A B --tabsize 4 Segmentation fault (core dumped) fnoyanisi@beastie:~ % uname -a FreeBSD beastie 12.1-RELEASE-p2 FreeBSD 12.1-RELEASE-p2 GENERIC amd64 I am working on a patch, checking out the latest head and building the world at the moment.