Bug 250824 - databases/postgresql13-server: incorrect checking of llvm versions
Summary: databases/postgresql13-server: incorrect checking of llvm versions
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: pgsql
URL:
Keywords:
: 250831 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-11-03 01:39 UTC by Dmitry Marakasov
Modified: 2021-05-17 08:11 UTC (History)
6 users (show)

See Also:
bugzilla: maintainer-feedback? (pgsql)


Attachments
Fix version detection and dependency (2.69 KB, patch)
2021-01-21 17:16 UTC, Olivier Certner
no flags Details | Diff
Fix COMPILER_VERSION uses (3.10 KB, patch)
2021-04-27 03:17 UTC, Jung-uk Kim
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Marakasov freebsd_committer 2020-11-03 01:39:47 UTC
The conditions to check LLVM version, e.g.

.if ${COMPILER_VERSION} > ${LLVM_DEFAULT}

and

.  if ${COMPILER_VERSION} <= 11

are not correct, because llvm version is not monotonic (60, 70, 80, 90, 10, 11)

Imo we should rename e.g. llvm10 to llvm100 to fix this properly and not overcomplicate logic here and in other places.
Comment 1 Palle Girgensohn freebsd_committer 2020-11-10 21:18:39 UTC
@brooks, what do you think? It would be very handy if version numbers for llvm where in fact matching what compiler.mk sets for clang.
Comment 2 Olivier Certner 2021-01-21 17:16:44 UTC
Created attachment 221794 [details]
Fix version detection and dependency

I stumbled on this as well. Please see the attached patch.

I'm not completely sure of what was exactly intended in the original code. The test on ${COMPILER_VERSION}, in particular, led me to think that using the base compiler had been considered as a possibility. However, without further patching of the configure part, this cannot work since there is no `llvm-config` in base (and LLVM in base may lack required components anyway, I don't know).

Apart from that, changes are pretty straightforward. They deal with non-monotonic versions. The algorithm is simply to try to use LLVM_DEFAULT, provided it is not greater than MAX_ALLOWED_MAJOR (set to 11, as before), in which case that one is used instead.

I agree that part of it (e.g., determination of major LLVM version) should ideally go into some infrastructure *.mk file.
Comment 3 Bernhard Froehlich freebsd_committer 2021-03-04 10:56:20 UTC
Can we make some progress on this please? On 13.0 we also install llvm11 though we have the same llvm version in base - this looks at least questionable.
Comment 4 Palle Girgensohn freebsd_committer 2021-03-04 14:35:38 UTC
I'm pretty sure that llvm is not installed in full on FreeBSD-12. As Olivier writes, required components are missing AFAIK. Hence, we need to install llvm from ports anyway.

I'm planning to bump a machine to FreeBSD-13 and get poudriere running there, so I can verify alls this properly. Missing llvm-config could be worked around, but that is probably not all that's missing.

Anyway, I guess the patch could be applied right now and would mitigate against installing a newer version of clang from ports, right?

Palle
Comment 5 Jung-uk Kim freebsd_committer 2021-04-27 03:17:56 UTC
Created attachment 224462 [details]
Fix COMPILER_VERSION uses

I had the exactly same issue and I wrote this patch without knowing this PR.  Because I don't know author's intention, I just tried to fix logic errors, i.e., COMPILER_VERSION vs. LLVM_DEFAULT comparison, etc.  Therefore, it is orthogonal to the previous patch.
Comment 6 Palle Girgensohn freebsd_committer 2021-05-14 23:35:45 UTC
*** Bug 250831 has been marked as a duplicate of this bug. ***
Comment 7 commit-hook freebsd_committer 2021-05-15 09:13:22 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=ab83f2b4bb78a718efa5c43247ba1e1d207f99b6

commit ab83f2b4bb78a718efa5c43247ba1e1d207f99b6
Author:     Palle Girgensohn <girgen@FreeBSD.org>
AuthorDate: 2021-05-15 09:11:12 +0000
Commit:     Palle Girgensohn <girgen@FreeBSD.org>
CommitDate: 2021-05-15 09:12:17 +0000

    databases/postgresql??-*: Upgrade to latest version

    PostgreSQL 13.3, 12.7, 11.12, 10.17, and 9.6.22 Released!

    The PostgreSQL Global Development Group has released an update to all supported
    versions of our database system, including 13.3, 12.7, 11.12, 10.17, and
    9.6.22. This release closes three security vulnerabilities and fixes over 45
    bugs reported over the last three months.

    Security fixes in this release:

    CVE-2021-32027: Buffer overrun from integer overflow in array subscripting
                    calculations

    CVE-2021-32028: Memory disclosure in INSERT ... ON CONFLICT ... DO UPDATE

    CVE-2021-32029: Memory disclosure in partitioned-table UPDATE ... RETURNING

    Also plenty of bug fixes. See the release note for details.

    Changes to the port:

    Make sure we use the matching version of llvm. This fixes a problem with the
    llvm version string not being monotonically increasing with the version
    number. [1]

    Better pkg message about checksums for postgresql 12+. [2] [4]

    Adjust login class parameter to adhere to the documentation in rc.subr(8) [3]:
      The rc.conf parameter for the login class of the postgresql daemon has
      changed name from postgresql_class to postgresql_login_class, since
      rc.subr(8) states that the parameter should be named ${name}_login_class.

    Allow parallel builds. [5]

    Correct the directory name for the user postgres in pkg message. [6]

    PR:             250824 [1], 253558 [2], 236060 [3], 233106 [4],  230656 [5]
    PR:             226674 [6]
    Submitted by:   Michael Zhilin [2], Michael Zhilin [3], Dmitry Chestnykh [4]
    Submitted by:   Steve Wills [5], knezour [6]

    Security:       76e0bb86-b4cb-11eb-b9c9-6cc21735f730
    Security:       62da9702-b4cc-11eb-b9c9-6cc21735f730

    Release notes:  https://www.postgresql.org/docs/release/

 UPDATING                                           |  8 ++++++
 databases/postgresql10-server/Makefile             |  4 +--
 databases/postgresql10-server/distinfo             |  6 ++---
 .../files/pkg-message-server.in                    |  8 +++---
 databases/postgresql10-server/files/postgresql.in  |  8 +++---
 databases/postgresql11-server/Makefile             |  4 +--
 databases/postgresql11-server/distinfo             |  6 ++---
 .../files/pkg-message-server.in                    |  8 +++---
 databases/postgresql11-server/files/postgresql.in  |  8 +++---
 databases/postgresql12-server/Makefile             |  4 +--
 databases/postgresql12-server/distinfo             |  6 ++---
 .../files/pkg-message-server.in                    | 14 +++++-----
 databases/postgresql12-server/files/postgresql.in  |  8 +++---
 databases/postgresql13-server/Makefile             | 31 +++++++++++++++-------
 databases/postgresql13-server/distinfo             |  6 ++---
 .../files/pkg-message-server.in                    | 14 +++++-----
 databases/postgresql13-server/files/postgresql.in  |  8 +++---
 databases/postgresql13-server/pkg-plist-client     |  1 +
 databases/postgresql13-server/pkg-plist-server     |  8 ++++++
 .../files/pkg-message-server.in                    |  8 +++---
 databases/postgresql96-server/Makefile             |  4 +--
 databases/postgresql96-server/distinfo             |  6 ++---
 .../files/pkg-message-server.in                    |  8 +++---
 databases/postgresql96-server/files/postgresql.in  |  8 +++---
 24 files changed, 112 insertions(+), 82 deletions(-)
Comment 8 Palle Girgensohn freebsd_committer 2021-05-15 09:13:57 UTC
Committed. Thanks!
Comment 9 commit-hook freebsd_committer 2021-05-17 08:11:25 UTC
A commit in branch 2021Q2 references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=4ac52e0c203ea1ac3cd4dd51dd6dcc89678fe4ac

commit 4ac52e0c203ea1ac3cd4dd51dd6dcc89678fe4ac
Author:     Palle Girgensohn <girgen@FreeBSD.org>
AuthorDate: 2021-05-15 09:11:12 +0000
Commit:     Palle Girgensohn <girgen@FreeBSD.org>
CommitDate: 2021-05-17 08:06:59 +0000

    databases/postgresql??-*: Upgrade to latest version

    PostgreSQL 13.3, 12.7, 11.12, 10.17, and 9.6.22 Released!

    The PostgreSQL Global Development Group has released an update to all supported
    versions of our database system, including 13.3, 12.7, 11.12, 10.17, and
    9.6.22. This release closes three security vulnerabilities and fixes over 45
    bugs reported over the last three months.

    Security fixes in this release:

    CVE-2021-32027: Buffer overrun from integer overflow in array subscripting
                    calculations

    CVE-2021-32028: Memory disclosure in INSERT ... ON CONFLICT ... DO UPDATE

    CVE-2021-32029: Memory disclosure in partitioned-table UPDATE ... RETURNING

    Also plenty of bug fixes. See the release note for details.

    Changes to the port:

    Make sure we use the matching version of llvm. This fixes a problem with the
    llvm version string not being monotonically increasing with the version
    number. [1]

    Better pkg message about checksums for postgresql 12+. [2] [4]

    Adjust login class parameter to adhere to the documentation in rc.subr(8) [3]:
      The rc.conf parameter for the login class of the postgresql daemon has
      changed name from postgresql_class to postgresql_login_class, since
      rc.subr(8) states that the parameter should be named ${name}_login_class.

    Allow parallel builds. [5]

    Correct the directory name for the user postgres in pkg message. [6]

    PR:             250824 [1], 253558 [2], 236060 [3], 233106 [4],  230656 [5]
    PR:             226674 [6]
    Submitted by:   Michael Zhilin [2], Michael Zhilin [3], Dmitry Chestnykh [4]
    Submitted by:   Steve Wills [5], knezour [6]

    Security:       76e0bb86-b4cb-11eb-b9c9-6cc21735f730
    Security:       62da9702-b4cc-11eb-b9c9-6cc21735f730

    Release notes:  https://www.postgresql.org/docs/release/

    (cherry picked from commit ab83f2b4bb78a718efa5c43247ba1e1d207f99b6)

 UPDATING                                           |  8 ++++++
 databases/postgresql10-server/Makefile             |  2 +-
 databases/postgresql10-server/distinfo             |  6 ++---
 .../files/pkg-message-server.in                    |  8 +++---
 databases/postgresql10-server/files/postgresql.in  |  8 +++---
 databases/postgresql11-server/Makefile             |  2 +-
 databases/postgresql11-server/distinfo             |  6 ++---
 .../files/pkg-message-server.in                    |  8 +++---
 databases/postgresql11-server/files/postgresql.in  |  8 +++---
 databases/postgresql12-server/Makefile             |  2 +-
 databases/postgresql12-server/distinfo             |  6 ++---
 .../files/pkg-message-server.in                    | 14 +++++------
 databases/postgresql12-server/files/postgresql.in  |  8 +++---
 databases/postgresql13-server/Makefile             | 29 ++++++++++++++++------
 databases/postgresql13-server/distinfo             |  6 ++---
 .../files/pkg-message-server.in                    | 14 +++++------
 databases/postgresql13-server/files/postgresql.in  |  8 +++---
 databases/postgresql13-server/pkg-plist-client     |  1 +
 databases/postgresql13-server/pkg-plist-server     |  8 ++++++
 .../files/pkg-message-server.in                    |  8 +++---
 databases/postgresql96-server/Makefile             |  2 +-
 databases/postgresql96-server/distinfo             |  6 ++---
 .../files/pkg-message-server.in                    |  8 +++---
 databases/postgresql96-server/files/postgresql.in  |  8 +++---
 24 files changed, 107 insertions(+), 77 deletions(-)