lib/59559: libc needs namespace protection for RENAME'd symbols

[kre%munnari.OZ.AU@localhost]

>Number:         59559
>Category:       lib
>Synopsis:       libc needs namespace protection for RENAME'd symbols
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jul 27 13:45:00 +0000 2025
>Originator:     Robert Elz
>Release:        All of them
>Organization:
>Environment:
Irrelevant
>Description:
	Note this is an offshoot of PR toolchain/59549 so you can look
	at that one to get the history - just ignore everything about
	both gdb and ctype(3) (the two subjects of that PR) as none of
	that is relevant here, and concentrate only on the reason that
	gdb was needed in the first place for that PR.

	Also see src/lib/libc/README for information on how the libc
	namespace & __RENAME() functionality is intended to work.

	And last of the preamble here, note that a lot of this is just
	my impressions, and might all be wrong, so if you know better,
	please explain the errors.

	What that README doesn't mention (and should) is that almost
	every libc global symbol which is referenced (as distinct from
	just defined) inside libc everywhere, needs to undergo the
	namespace dance, as described there for "NetBSD-specific
	nonstandard extensions" (even when they aren't) as otherwise
	a call in libc to what should be a standard function can be
	overridden by an application which doesn't use that symbol,
	doesn't include any header which defines it, and hence is
	permitted to redefine the symbol however it likes.

	But that's not the point of this PR, just a side comment.

	It is the third type of libc function (in the README, in the
	section "Old versions of library routines") which is the
	issue here.

	I will briefly summarise the example given there so I can
	expand on it more:

	There once was a function (well there still is)
		time_t  time(time_t *);
	but once upon a time, time_t was int (aka int32_t)
	making that function effectively:
		int32_t time(int32_t *);
	but the definition of time_t was changed, so now
	we actually need
		int64_t time(int64_t *);

	In olden timed we had
		_time() (from namespace.h) for the code
		time	(as a weak alias in libc).

	With that, any really old code that called time() from
	libc actually ended up calling _time() - the 32 bit
	version of that interface - yet if an application doesn't
	care about that interface, doesn't include <time.h> nor
	any other header which defines it, it can define its own
	global "time" (as a global variable, or a function),
	which it can use, while anything inside libc calls _time()
	and all is good - this is how things are supposed to work.

	(And note, that up to that point, the signature of time()
	had never [within the meaning of "never" here] changed,
	so according to the README, the _time() variant from
	namespace wouldn't be needed - but it is, and was, and
	was all done correctly, it is just the README that is
	lacking here.)

	Then we changed time_t, and as README says, libc now
	provides

        _time           global (implementing the old signature)
        time            weak alias for _time
        __time50        global (implementing the new signature)

	which all looks good, both _time and __time50 (for the "50"
	see the README, that's not especially relevant here) are
	_* symbols, which applications are not supposed to use,
	those are reserved to the implementation, so no application
	should be defining a global symbol __time50.

	Or you'd think.

	But the application still calls "time(...)", and
	compiler/linker magic make new calls turn into calls to
	__time50() (that's OK, that's the implementation doing
	that, so use of the _* symbol is permitted).   That's
	what makes all of this work for your normal application.

	But the application can also redefine time to be something
	else - except this now turns into a bad example to use,
	as time() is a standard function, defined in <time.h> and
	if the application has included <time.h> it is not supposed
	to redefine any symbols that header makes available.

	So let's switch to the other case "NetBSD-specific nonstandard
	extensions" - those things can be subject to signature changes
	as much as anything else, and for this use the example from the
	original PR (what caused all the problems) gmtime_r -- this is
	a POSIX standard function, but not a C standard function (which
	kind of makes it a weird special case, but still).

	That one got internally renamed (to __gmtime_r50) at the same
	time as time() was renamed above.

	But as gmtime_r isn't a standard C function, an application
	which wants to use it might very easily want to define a version
	of it, just in case the implementation does not provide that
	function (which as it happens, some do not, I believe, despite
	it being required by POSIX).

	As the function is intended to implement the (POSIX) standard
	gmtime_r function, it is reasonable to get its declaration from
	<time.h> which is where POSIX says it should be (though that is
	probably fairly stupid, as if the declaration is there, and
	visible, the implementation should be available as well).

	Anyway, if the application does define a gmtime_r() function,
	the renaming magic that accompanies the declaration of that
	function (as the original gmtime_r is the 32 bit version, and
	no new code wants that any more) applies to the definition of
	the function (just as it does - as the README explains - to the
	implementation inside libc).

	That is, by providing a mktime_r function definition, what the
	application has actually done (with implementation assistance)
	is to define a function called __mktime_r50().   That would be
	OK, except internally libc calls mktime_r (it is used to implement
	mktime) so now we have the libc internals calling the application's
	function - from which good results should not be expected (and
	indeed, they were not - the implementation has mktime() calling
	mktime_r() - the application has mktime_r() calling mktime(), using
	the libc version of that, which is of course, simply infinite
	recursion, hence the application crashed).

	We could avoid that by treating all RENAME'd symbols (which always
	rename into implementation namespace of course) as if they were
	global symbols in the application namespace, and doing the namespace
	dance for them as well -- meaning that in this case, __gmtime_r50
	would be ___gmtime_r50, and that's what would be called inside libc,
	and for external callers, there'd be a weak alias, which would map
	__gmtime_r50 into ___gmtime_r50 for any application which does not
	define its own __gmtime_r50 (ie: gmtime_r).

	That is, we could, but should we?   The __RENAME() is only visible
	in the header along with the declaration of the function being
	renamed, and if that is there, and the application has made it
	visible, do the application should not be redefining the function.
	So we could just leave things alone, and punt any issues caused
	by this back as application bugs (which really they are).  If the
	application in this case had simply defined mktime_r without including
	<time.h> if that were possible (in this case, I believe not), or or
	explicitly requested strict Cxx (doesn't matter which version)
	then it would not have encountered the __RENAME() and it all would
	have just worked.

	[Note: for the application in question doing all this was probably
	insane anyway, as its gmtime_r() implementation was decidedly inferior
	to the libc version, and in particular, was not thread safe, in an
	application which, I believe, uses threads.]

>How-To-Repeat:
	See PR toolchain/59549    But don't bother trying to make it happen.

>Fix:
	We could add a bunch more to src/lib/libc/include/namespace.h
	but should we?