Bug 211873

Summary: lib/libc/gen/basedirname_test:dirname_posix fails: "Input buffer was modified."
Product: Base System Reporter: Enji Cooper <ngie>
Component: binAssignee: Ed Schouten <ed>
Status: Closed FIXED    
Severity: Affects Some People CC: emaste, ngie
Priority: --- Flags: ngie: mfc-stable11-
ngie: mfc-stable10-
Version: CURRENT   
Hardware: Any   
OS: Any   

Description Enji Cooper freebsd_committer freebsd_triage 2016-08-15 17:24:13 UTC
Looks like the behavior for dirname(3) on FreeBSD deviates from NetBSD. Bringing this up because we may or may not want to follow suit.

Assigning to ed@ because he's been heading up the recent basename(3)/dirname(3) POSIXification.

$ svn info /usr/src/svn/
Path: /usr/src/svn
Working Copy Root Path: /usr/src/svn
URL: svn+ssh://ngie@repo.freebsd.org/base/projects/netbsd-tests-update-12
Relative URL: ^/projects/netbsd-tests-update-12
Repository Root: svn+ssh://ngie@repo.freebsd.org/base
Repository UUID: ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
Revision: 304099
Node Kind: directory
Schedule: normal
Last Changed Author: ngie
Last Changed Rev: 304099
Last Changed Date: 2016-08-14 12:54:40 -0700 (Sun, 14 Aug 2016)

$ kyua debug -k lib/libc/gen/Kyuafile basedirname_test:dirname_posix
Input buffer for "/usr/lib" was modified
basedirname_test:dirname_posix  ->  failed: Input buffer was modified.

165                 /*
166                  * dirname(3) is allowed to modify the input buffer.
167                  * However, that is considered hostile by some programs,
168                  * and so we elect to consider this an error.
169                  *
170                  * This is not a problem, as dirname(3) is also allowed
171                  * to return a pointer to a statically-allocated buffer
172                  * (it is explicitly not required to be reentrant).
173                  */
174                 if (test_dirname_table[i].input != NULL &&
175                     strcmp(test_dirname_table[i].input, testbuf) != 0) {
176                         fprintf(stderr,
177                             "Input buffer for \"%s\" was modified\n",
178                             test_dirname_table[i].input);
179                         atf_tc_fail("Input buffer was modified.");
180                 }
Comment 1 commit-hook freebsd_committer freebsd_triage 2016-08-15 19:06:26 UTC
A commit references this bug:

Author: ed
Date: Mon Aug 15 19:05:42 UTC 2016
New revision: 304175
URL: https://svnweb.freebsd.org/changeset/base/304175

Log:
  Disable tests for non-standard behaviour of dirname(3)/basename(3).

  The NetBSD ATF tests explicitly check that these functions do not modify
  their input. These tests are NetBSD-specific. They test for something
  that is not part of POSIX.

  PR:		211873
  Reviewed by:	ngie
  Differential Revision:	https://reviews.freebsd.org/D7506

Changes:
  head/contrib/netbsd-tests/lib/libc/gen/t_basedirname.c
Comment 2 Enji Cooper freebsd_committer freebsd_triage 2016-08-15 20:49:26 UTC
No rush on the MFCs... I can commit the changes if desired.
Comment 3 Enji Cooper freebsd_committer freebsd_triage 2016-08-15 21:33:58 UTC
Reopening and putting in-progress for necessary MFC work.
Comment 4 Ed Schouten freebsd_committer freebsd_triage 2016-08-16 12:32:24 UTC
This change doesn't need to be MFCed, right? The changes to basename()/dirname() themselves are not going to be merged back either.
Comment 5 Enji Cooper freebsd_committer freebsd_triage 2016-08-16 17:35:36 UTC
(In reply to Ed Schouten from comment #4)

Not much is lost in merging this change. It helps downstream consumers if they decide to take the basename/dirname API change.
Comment 6 Ed Schouten freebsd_committer freebsd_triage 2016-08-17 08:02:08 UTC
Well, I think it actually makes sense not to MFC it. The changes I'm making to basename(3)/dirname(3) should not be MFCed under any circumstance. FreeBSD<=11 should simply stick to the previously existing semantics.

By not merging back the changes to these tests, we can be sure this won't happen. Both in stable/11 and in trunk, we are testing exactly the behaviour that we expect.
Comment 7 commit-hook freebsd_committer freebsd_triage 2017-02-09 09:03:24 UTC
A commit references this bug:

Author: ngie
Date: Thu Feb  9 09:02:46 UTC 2017
New revision: 313471
URL: https://svnweb.freebsd.org/changeset/base/313471

Log:
  MFC r288444,r296586,r301753,r303840,r304175:

  r288444 (by andrew):

  Pass 8 arguments to makecontext on arm64 as this is all we support.

  Obtained from:	EuroBSDCon Devsummit

  r296586 (by bdrewery):

  Fix and connect setjmp test.

  r301753:

  Fix up r274061

  Detect /usr/share/dict/words the "right way" by using require.files instead of
  the hacked up attempt in the dict(..) function, which didn't work properly on
  systems where MK_DICT == no.

  r303840 (by jhb):

  Add timer_settime tests using SIGEV_THREAD.

  Note that these tests should work fine on NetBSD and other systems as
  SIGEV_THREAD is POSIX.

  r304175 (by ed):

  Disable tests for non-standard behaviour of dirname(3)/basename(3).

  The NetBSD ATF tests explicitly check that these functions do not modify
  their input. These tests are NetBSD-specific. They test for something
  that is not part of POSIX.

  PR:		211873

Changes:
_U  projects/stable-10-backport-test-changes/
  projects/stable-10-backport-test-changes/contrib/netbsd-tests/lib/libc/db/t_db.sh
  projects/stable-10-backport-test-changes/contrib/netbsd-tests/lib/libc/gen/t_basedirname.c
  projects/stable-10-backport-test-changes/contrib/netbsd-tests/lib/libc/setjmp/t_setjmp.c
  projects/stable-10-backport-test-changes/contrib/netbsd-tests/lib/libc/setjmp/t_threadjmp.c
  projects/stable-10-backport-test-changes/contrib/netbsd-tests/lib/libc/sys/t_getcontext.c
  projects/stable-10-backport-test-changes/contrib/netbsd-tests/lib/libc/sys/t_timer_create.c
  projects/stable-10-backport-test-changes/etc/mtree/BSD.tests.dist
  projects/stable-10-backport-test-changes/lib/libc/tests/Makefile