Created attachment 215384 [details] Just a small patch The function getlogin_r() is declared with a second parameter of type int. This surely was meant to be a size_t. See also: contrib/llvm-project/llvm/include/llvm/Analysis/TargetLibraryInfo.def:889:/// int getlogin_r(char *name, size_t namesize) Ruby 2.8 compilation was careried out on Linux, but breaks on FreeBSD because of this.
^Triage: - brooks@ last touched around this area in base r332119 and may have some input on this, request feedback - wes@ originally wrote it, do the same
These functions are specified by POSIX as... taking size_t. Yeah, our version seems wrong. However, our libc implementation takes int, so to change this you need to provide libc ABI compatibility for the old version.
Something like this: --- a/lib/libc/gen/Symbol.map +++ b/lib/libc/gen/Symbol.map @@ -156,7 +156,6 @@ FBSD_1.0 { gethostname; getloadavg; getlogin; - getlogin_r; setnetgrent; getnetgrent; endnetgrent; @@ -423,6 +422,7 @@ FBSD_1.5 { }; FBSD_1.6 { + getlogin_r; memalign; sigandset; sigisemptyset; --- a/lib/libc/gen/getlogin.c +++ b/lib/libc/gen/getlogin.c @@ -58,7 +58,7 @@ getlogin(void) } int -getlogin_r(char *logname, int namelen) +getlogin_r(char *logname, size_t namelen) { char tmpname[MAXLOGNAME]; int len; @@ -75,3 +75,13 @@ getlogin_r(char *logname, int namelen) strlcpy(logname, tmpname, len); return (0); } + +/* FreeBSD 12 and earlier compat. */ +int +__getlogin_r_fbsd12(char *logname, int namelen) +{ + if (namelen < 1) + return (ERANGE); + return (getlogin_r(logname, namelen)); +} +__sym_compat(getlogin_r, __getlogin_r_fbsd12, FBSD_1.0);
I posted this bug report almost three months ago. In the meantime, the fix works here through the whole time. Will there ever happen anything further? Do you want to get rid of me? Sorry for trying to contribute. If this post won't receive an answer, I will file another bug report about the bug report system.
(In reply to Bertram Scharpf from comment #4) Hi, Sorry, but there was no confirmation that Conrad's patch actually fixed the problem that spurred this PR. He fixed the problem he identified with the standard mismatch, but he received no feedback to confirm that there aren't any other unexpected issues (e.g. neither patch actually fixes the declaration in ^/include/unistd.h, for instance) -- based on this alone, I wouldn't have picked it up either.
So, what has to happen that I can remove my local patch and still compile Ruby2.8?
(In reply to Bertram Scharpf from comment #6) I've pushed the complete patch (your manage change + cem's ABI compat patch + header change) into phabricator at the above-referenced review URL. Once it's gotten another review or two, I'll commit it and MFC in a couple days to get it into 12.2.
Thank you. This is the third time I was forgotten.
To the original reporter, have you tried the combined patch as proposed (in D26335)?
A commit references this bug: Author: kevans Date: Wed Sep 9 18:07:14 UTC 2020 New revision: 365506 URL: https://svnweb.freebsd.org/changeset/base/365506 Log: getlogin_r: fix the type of len getlogin_r is specified by POSIX to to take a size_t len, not int. Fix our version to do the same, bump the symbol version due to ABI change and provide compat. This was reported to break compilation of Ruby 2.8. Some discussion about the necessity of the ABI compat did take place in the review. While many 64-bit platforms would likely be passing it in a 64-bit register and zero-extended and thus, not notice ABI breakage, some do sign-extend (e.g. mips). PR: 247102 Submitted by: Bertram Scharpf <software@bertram-scharpf.de> (original) Submitted by: cem (ABI compat) MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D26335 Changes: head/include/unistd.h head/lib/libc/gen/Symbol.map head/lib/libc/gen/getlogin.c head/lib/libc/sys/getlogin.2
A commit references this bug: Author: kevans Date: Sun Sep 13 01:44:32 UTC 2020 New revision: 365682 URL: https://svnweb.freebsd.org/changeset/base/365682 Log: MFC r365506 getlogin_r: fix the type of len getlogin_r is specified by POSIX to to take a size_t len, not int. Fix our version to do the same, bump the symbol version due to ABI change and provide compat. This was reported to break compilation of Ruby 2.8. Some discussion about the necessity of the ABI compat did take place in the review. While many 64-bit platforms would likely be passing it in a 64-bit register and zero-extended and thus, not notice ABI breakage, some do sign-extend (e.g. mips). PR: 247102 Changes: _U stable/12/ stable/12/include/unistd.h stable/12/lib/libc/gen/Symbol.map stable/12/lib/libc/gen/getlogin.c stable/12/lib/libc/sys/getlogin.2
A commit references this bug: Author: kevans Date: Sun Sep 13 02:17:59 UTC 2020 New revision: 365684 URL: https://svnweb.freebsd.org/changeset/base/365684 Log: MFS r365682: getlogin_r: fix the type of len getlogin_r is specified by POSIX to to take a size_t len, not int. Fix our version to do the same, bump the symbol version due to ABI change and provide compat. This was reported to break compilation of Ruby 2.8. Some discussion about the necessity of the ABI compat did take place in the review. While many 64-bit platforms would likely be passing it in a 64-bit register and zero-extended and thus, not notice ABI breakage, some do sign-extend (e.g. mips). PR: 247102 Approved by: re (gjb) Changes: _U releng/12.2/ releng/12.2/include/unistd.h releng/12.2/lib/libc/gen/Symbol.map releng/12.2/lib/libc/gen/getlogin.c releng/12.2/lib/libc/sys/getlogin.2
Merged all the way to releng/12.2; thanks!