Bug 280784 - libmd: exp-run for new symbol versioning
Summary: libmd: exp-run for new symbol versioning
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Kyle Evans
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-08-12 22:21 UTC by Robert Clausecker
Modified: 2024-10-01 15:29 UTC (History)
4 users (show)

See Also:
antoine: exp-run+


Attachments
D34497+D34498+D34499+D34500+D34501+D34502+D34503+some (82.69 KB, patch)
2024-08-12 22:21 UTC, Robert Clausecker
no flags Details | Diff
patch: rename stack_protector.c, consolidate md4c.c (16.07 KB, patch)
2024-08-14 15:23 UTC, Robert Clausecker
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Clausecker freebsd_committer freebsd_triage 2024-08-12 22:21:18 UTC
Created attachment 252722 [details]
D34497+D34498+D34499+D34500+D34501+D34502+D34503+some

The attached patch set was proposed by kevans@ to introduce symbol-versioning to libmd.  There's also a general clean up of the API to only what is declared in the headers and some refactor.

Please do an exp-run of this patch set against the ports tree to determine if any of the symbols that are now hidden are required by some random software we ship in ports.

Patch set should apply to CURRENT.
Comment 1 Antoine Brodin freebsd_committer freebsd_triage 2024-08-13 19:53:23 UTC
buildworld fails:

In file included from /poudriere/data/src-PR280784/sys/kern/stack_protector.c:4:
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/kernel.h:221:2: error: type name requires a specifier or qualifier
  221 |         STAILQ_ENTRY(sysinit)   next;           /* singly-linked list */
      |         ^
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/kernel.h:221:15: error: a parameter list without types is only allowed in a function definition
  221 |         STAILQ_ENTRY(sysinit)   next;           /* singly-linked list */
      |                      ^
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/kernel.h:221:23: error: expected ';' at end of declaration list
  221 |         STAILQ_ENTRY(sysinit)   next;           /* singly-linked list */
      |                              ^
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/kernel.h:471:2: error: type name requires a specifier or qualifier
  471 |         STAILQ_ENTRY(intr_config_hook) ich_links;
      |         ^
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/kernel.h:471:15: error: a parameter list without types is only allowed in a function definition
  471 |         STAILQ_ENTRY(intr_config_hook) ich_links;
      |                      ^
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/kernel.h:471:32: error: expected ';' at end of declaration list
  471 |         STAILQ_ENTRY(intr_config_hook) ich_links;
      |                                       ^
In file included from /poudriere/data/src-PR280784/sys/kern/stack_protector.c:6:
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/libkern.h:82:15: error: unknown type name 'bool'
   82 | static inline bool
      |               ^
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/libkern.h:262:7: error: incompatible redeclaration of library function 'strdup' [-Werror,-Wincompatible-library-redeclar
ation]
  262 | char    *strdup(const char *__restrict, struct malloc_type *);
      |          ^
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/libkern.h:262:7: note: 'strdup' is a builtin with type 'char *(const char *)'
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/libkern.h:264:7: error: incompatible redeclaration of library function 'strndup' [-Werror,-Wincompatible-library-redecla
ration]
  264 | char    *strndup(const char *__restrict, size_t, struct malloc_type *);
      |          ^
/usr/obj/poudriere/data/src-PR280784/amd64.amd64/tmp/usr/include/sys/libkern.h:264:7: note: 'strndup' is a builtin with type 'char *(const char *, unsigned long)'
/poudriere/data/src-PR280784/sys/kern/stack_protector.c:15:2: error: call to undeclared function 'panic'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit
-function-declaration]
   15 |         panic("stack overflow detected; backtrace may be corrupted");
      |         ^
10 errors generated.
*** [stack_protector.o] Error code 1
Comment 2 Robert Clausecker freebsd_committer freebsd_triage 2024-08-14 00:24:19 UTC
(In reply to Antoine Brodin from comment #1)

That is very weird.  What commit did you apply this patch set to?  I'll look into it.
Comment 3 Robert Clausecker freebsd_committer freebsd_triage 2024-08-14 15:23:58 UTC
Created attachment 252758 [details]
patch: rename stack_protector.c, consolidate md4c.c

Please also apply the attached patches courtesy of kevans@.  I dropped them from the patch set as I had thought they were unrelated.

With them included, the build should hopefully succeed.
Comment 4 Antoine Brodin freebsd_committer freebsd_triage 2024-08-20 09:47:53 UTC
Exp-run looks fine
Comment 5 Robert Clausecker freebsd_committer freebsd_triage 2024-08-20 21:39:15 UTC
Assign to author for commit.
Comment 6 commit-hook freebsd_committer freebsd_triage 2024-09-30 03:37:06 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=442e0975ee4b3d6ea809359b7da670b7bd548435

commit 442e0975ee4b3d6ea809359b7da670b7bd548435
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-03-23 02:39:02 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-09-30 03:34:21 +0000

    Consolidate md4 implementations written in C

    We currently have one in libmd and another in the kernel that's almost
    completely identical.  Standardize on the kernel version.

    PR:             280784 (exp-run)

 lib/libmd/md4.h         |  55 +--------
 lib/libmd/md4c.c (gone) | 299 ------------------------------------------------
 sys/kern/md4c.c         |  19 +++
 sys/sys/md4.h           |  48 +++++++-
 4 files changed, 64 insertions(+), 357 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2024-09-30 03:37:07 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=01112a1711f3b7f329d84f7946ee0b8cdd1872c9

commit 01112a1711f3b7f329d84f7946ee0b8cdd1872c9
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-03-08 17:20:09 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-09-30 03:34:20 +0000

    libmd: tests: raise WARNS to the default

    The drivers just had a small issue, passing a literal string as
    non-const.  Fix it and lift WARNS.

    PR:             280784 (exp-run)
    Reviewed by:    delphij, emaste
    Differential Revision:  https://reviews.freebsd.org/D34501

 lib/libmd/tests/Makefile      | 3 ---
 lib/libmd/tests/mddriver.c    | 4 ++--
 lib/libmd/tests/rmddriver.c   | 4 ++--
 lib/libmd/tests/shadriver.c   | 2 +-
 lib/libmd/tests/skeindriver.c | 2 +-
 5 files changed, 6 insertions(+), 9 deletions(-)
Comment 8 commit-hook freebsd_committer freebsd_triage 2024-09-30 03:37:08 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=d61f4b481f4876a8640830f9b06c8ffa2bdcae7d

commit d61f4b481f4876a8640830f9b06c8ffa2bdcae7d
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-03-08 15:52:01 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-09-30 03:34:18 +0000

    libmd: stop exporting _block symbols

    These are needed across compilation units so we can keep the _libmd_
    prefixing bits (though I suspect we're not likely to collide), but we
    don't need to be exporting the unprefixed versions of these; it's an
    implementation detail.

    PR:             280784 (exp-run)
    Reviewed by:    delphij, fuz
    Differential Revision:  https://reviews.freebsd.org/D34498

 lib/libmd/rmd160c.c | 2 --
 lib/libmd/sha1c.c   | 4 ----
 2 files changed, 6 deletions(-)
Comment 9 commit-hook freebsd_committer freebsd_triage 2024-09-30 03:37:09 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=fd3ced15070885c818b74a44a0fbe45ed8687f44

commit fd3ced15070885c818b74a44a0fbe45ed8687f44
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-03-08 17:42:52 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-09-30 03:34:20 +0000

    libmd: export and document *Fd/*FdChunk interfaces

    PR:             280784 (exp-run)
    Fixes: de13c2427d6c ("libmd: introduce functions that operate on an fd")
    Reviewed by:    manpages (bcr), fuz
    Differential Revision:  https://reviews.freebsd.org/D34502

 lib/libmd/mdX.3    | 21 ++++++++++++++++++++-
 lib/libmd/mdXhl.c  |  4 ++++
 lib/libmd/ripemd.3 | 21 ++++++++++++++++++++-
 lib/libmd/sha.3    | 27 ++++++++++++++++++++++++++-
 lib/libmd/sha256.3 | 27 ++++++++++++++++++++++++++-
 lib/libmd/sha512.3 | 33 ++++++++++++++++++++++++++++++++-
 lib/libmd/skein.3  | 33 ++++++++++++++++++++++++++++++++-
 7 files changed, 160 insertions(+), 6 deletions(-)
Comment 10 commit-hook freebsd_committer freebsd_triage 2024-09-30 03:37:10 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e7a629c851d747772cc138efcb0418809ecdea55

commit e7a629c851d747772cc138efcb0418809ecdea55
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-03-08 15:39:52 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-09-30 03:34:18 +0000

    libmd, kern, stand: consolidate md5 implementations (NFC)

    Reduce the number of md5c.c between the three of these from two to one
    by just reaching into the kernel build for both userland builds.  The
    precedent for this already exists for sha2 in both cases.

    _libmd_ symbol privatization bits have been moved to sys/md5.h and
    md5.h remains to #include <sys/md5.h> for compatibility.

    This stops exporting MD5Pad() in the process because the kernel stopped
    exporting it in 502a35d60f4c.  soversion is bumped accordingly.

    This also renames the libc version of stack_protector.c; it previously
    only worked by coincidence because .PATH ordering worked out such that
    we got the right one, but this is not the case anymore.  Remove the
    landmine.

    PR:             280784 (exp-run)
    Reviewed by:    allanjude, delphij
    Differential Revision:  https://reviews.freebsd.org/D34497

 ObsoleteFiles.inc                                  |   3 +
 lib/libc/Makefile                                  |   2 +-
 lib/libc/md/Makefile.inc                           |   2 +-
 lib/libc/secure/Makefile.inc                       |   2 +-
 .../{stack_protector.c => libc_stack_protector.c}  |   0
 lib/libcrypt/Makefile                              |   2 +-
 lib/libmd/Makefile                                 |   3 +-
 lib/libmd/md5.h                                    |  40 ---
 lib/libmd/md5c.c (gone)                            | 344 ---------------------
 lib/libssp/Makefile                                |   2 +-
 stand/libsa/Makefile                               |   4 +-
 sys/kern/md5c.c                                    |  23 +-
 sys/sys/md5.h                                      |  42 +++
 13 files changed, 72 insertions(+), 397 deletions(-)
Comment 11 commit-hook freebsd_committer freebsd_triage 2024-09-30 03:37:11 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e25527f75f73665517c7a449cdc6a29fa4c90c1c

commit e25527f75f73665517c7a449cdc6a29fa4c90c1c
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-03-08 16:12:19 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-09-30 03:34:19 +0000

    libmd: symbol versioning

    The primary benefit sought is exporting _libmd_* symbols in a private
    namespace, and avoiding export of some other implementation details that
    are shared amongst TUs.

    PR:             280784 (exp-run)
    Reviewed by:    fuz
    Differential Revision:  https://reviews.freebsd.org/D34499

 lib/libmd/Makefile         |   3 +
 lib/libmd/Symbol.map (new) | 272 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 275 insertions(+)
Comment 12 commit-hook freebsd_committer freebsd_triage 2024-09-30 03:37:12 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=81de655acd2ebdf104dc4a332eaee5e62b8440a2

commit 81de655acd2ebdf104dc4a332eaee5e62b8440a2
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-03-08 18:05:24 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-09-30 03:34:20 +0000

    libmd: stop exporting Transform() symbols

    They're not documented in libmd and we don't have any consumers.  It's
    problematic to keep them exported, as we don't currently export their
    implementations. Make them all private.

    PR:             280784 (exp-run)
    Reviewed by:    fuz
    Differential Revision:  https://reviews.freebsd.org/D34503

 lib/libmd/Symbol.map      | 11 -----------
 lib/libmd/ripemd.h        |  3 ---
 lib/libmd/rmd160c.c       |  4 +---
 lib/libmd/sha.h           |  6 ------
 lib/libmd/sha0c.c         |  2 +-
 lib/libmd/sha1c.c         |  6 +-----
 sys/crypto/sha2/sha256.h  |  3 ---
 sys/crypto/sha2/sha256c.c |  2 --
 sys/crypto/sha2/sha512.h  |  3 ---
 sys/crypto/sha2/sha512c.c |  2 --
 sys/crypto/sha2/sha512t.h |  6 ------
 sys/kern/md5c.c           |  2 --
 12 files changed, 3 insertions(+), 47 deletions(-)
Comment 13 commit-hook freebsd_committer freebsd_triage 2024-09-30 03:37:13 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e0c51286dd6d1c1ce6d3761933a028cc3dcdd6ca

commit e0c51286dd6d1c1ce6d3761933a028cc3dcdd6ca
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-03-08 17:16:13 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-09-30 03:34:19 +0000

    libmd: split tests out into the test infrastructure

    Make us a little less reliant on individuals running the tests, we'll
    start running them as part of CI.

    PR:             280784 (exp-run)
    Reviewed by:    delphij
    Differential Revision:  https://reviews.freebsd.org/D34500

 etc/mtree/BSD.tests.dist            |   2 +
 lib/libmd/Makefile                  | 245 +---------------------------------
 lib/libmd/tests/Makefile (new)      | 258 ++++++++++++++++++++++++++++++++++++
 lib/libmd/{ => tests}/mddriver.c    |   0
 lib/libmd/{ => tests}/rmddriver.c   |   0
 lib/libmd/{ => tests}/shadriver.c   |   0
 lib/libmd/{ => tests}/skeindriver.c |   0
 7 files changed, 262 insertions(+), 243 deletions(-)
Comment 14 Robert Clausecker freebsd_committer freebsd_triage 2024-10-01 15:29:54 UTC
All changes have been committed.

One problem went annoticed before: pkg-static now no longer builds on arm64 due to a symbol conflict.  kevans@ is investigating this one.