[SERVER-45922] Build failure in overflow_arithmetic.h: constexpr function never produces a constant expression Created: 02/Feb/20  Updated: 29/Oct/23  Resolved: 06/Feb/20

Status: Closed
Project: Core Server
Component/s: Build
Affects Version/s: 4.2.3
Fix Version/s: 4.2.4, 4.3.4

Type: Bug Priority: Major - P3
Reporter: Ralph Seichter Assignee: Andrew Morrow (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File main.log    
Issue Links:
Backports
Backwards Compatibility: Fully Compatible
Operating System: ALL
Backport Requested:
v4.2
Steps To Reproduce:
  • Install MacPorts on your High Sierra based (with all available Apple updates) Mac.
  • Run sudo port install mongodb
Sprint: Dev Platform 2020-02-10
Participants:

 Description   

The attempt to build MongoDB 4.2.3 on macOS 10.14 (High Sierra) fails:

:info:build src/mongo/platform/overflow_arithmetic.h:97:16: error: constexpr function never produces a constant expression [-Winvalid-constexpr]
:info:build constexpr bool mongoSignedMultiplyOverflow64(long lhs, long rhs, long* product) {
:info:build ^
:info:build src/mongo/platform/overflow_arithmetic.h:98:12: note: subexpression not valid in a constant expression
:info:build return __builtin_mul_overflow(lhs, rhs, product);
:info:build ^

Please also see attached build log and MacPorts ticket 60032



 Comments   
Comment by Githook User [ 06/Feb/20 ]

Author:

{'name': 'Andrew Morrow', 'username': 'acmorrow', 'email': 'acm@mongodb.com'}

Message: SERVER-45922 Bump required macOS Xcode version fom 10.0 to 10.2

(cherry picked from commit 98159f8798a3d73878beab634e9ce81784848e8f)
Branch: v4.2
https://github.com/mongodb/mongo/commit/fb89639d710efa9b5d7e6acb2ccf44a157d4ba1b

Comment by Andrew Morrow (Inactive) [ 06/Feb/20 ]

mongodb.com@seichter.de - Not a problem. As it turns out, there was a real bug in our build system. We will fix up the minimum so others don't get burned by this too.

Comment by Ralph Seichter [ 05/Feb/20 ]

Thanks for taking the time to explain things to me, and for addressing the underlying issue.

Comment by Ryan Schmidt [ 05/Feb/20 ]

And in case my comment earlier caused confusion, when I mentioned blacklisting a compiler I was referring to a feature of MacPorts by which a portfile author can specify Xcode clang versions with which that port will not work. We had been blacklisting clang < 1000 for mongodb. After what we've learned in this ticket, I've updated it to blacklist clang < 1001.

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

For all versions of MongoDB, across all the compilers (gcc, clang, Xcode, MSVC) relevant on each of our various platforms, we define in the build system official compiler minima which (should!) match our CI compilers for the branch/platform combination. For the v4.2 branch, that minimum on macOS was originally set at Xcode 10.0. However, at some point we apparently upgraded our CI macOS compilers to Xcode 10.2 but failed to upgrade the enforced compiler minimum to match. Since we were building with Xcode 10.2 we never noticed that v4.2 was broken with Xcode 10.1. I don't happen to have Xcode 10.0 handy to test, but it is also possible that this was a regression in Xcode 10.1 and that Xcode 10.0 would be fine, or it is possible that Xcode 10 is also broken. If you have Xcode 10.0 installed you could easily test it out using my repro above.

We set these minima so that users building MongoDB from source don't find themselves building with a toolchain that is definitely not going to work, encountering a build break, and then filing tickets to us that we end up closing as "use a newer compiler". However, we do offer the --disable-minimum-compiler-version-enforcement flag to allow these minima to be ignored in situations where it is warranted. We really don't recommend that though. I'm not sure how good C++17 support is in Xcode 9, and we do no testing of the v4.2 database with it. It is possible that an Xcode 9 build of v4.2 will compile and link but have erroneous runtime behavior.

We don't currently have the ability to blacklist specific versions. We just set a minimum. Usually these minima are upgraded in lockstep across all platforms when we do a C++ language standard upgrade which requires a new toolchain to support. MongoDB v4.2 was the first build to require C++17, and we upgraded the compiler minima for MSVC, GCC, clang, and Xcode simultaneously. Adding support for a blacklist would be tacitly claiming support for older compiler versions, but without explicitly testing them, it would be risky. And explicitly testing them would massively expand the amount of testing we need to perform.

Regarding clang 9, Xcode 10 appears to be more or less clang 7, not clang 9. To somewhat prove this, the -fsanitize=implicit-integer-sign-change is new in clang 8: https://releases.llvm.org/8.0.0/tools/clang/docs/ReleaseNotes.html#undefined-behavior-sanitizer-ubsan

Using homebrew clang 7 and clang 8, we can see that clang 7 doesn't know about this flag, but clang 8 does:

$ /usr/local/opt/llvm@7/bin/clang++ -fsanitize=implicit-integer-sign-change ./hello_world.cpp
clang-7: error: unsupported argument 'implicit-integer-sign-change' to option 'fsanitize='
 
$ /usr/local/opt/llvm@8/bin/clang++ -fsanitize=implicit-integer-sign-change ./hello_world.cpp && echo ok
ok

If we try this with various Xcode, we get interesting results:

$ DEVELOPER_DIR=/Applications/Xcode10.2.app xcrun clang++ -fsanitize=implicit-integer-sign-change ./hello_world.cpp
clang: error: unsupported argument 'implicit-integer-sign-change' to option 'fsanitize='
 
$ DEVELOPER_DIR=/Applications/Xcode11.3.app xcrun clang++ -fsanitize=implicit-integer-sign-change ./hello_world.cpp
clang: error: unsupported option '-fsanitize=implicit-integer-sign-change' for target 'x86_64-apple-darwin18.7.0'

Now what is interesting here is that they both errored out, but Xcode 11 is at least aware of the flag and knows it isn't OK on this target. So, if you consider this experiment valid, Xcode 10 cannot be newer than clang 7 (or, is definitely older than clang 8), and Xcode 11 is at least clang 8, but could be newer. It is incredibly unfortunate that Apple makes it so obscure what the relationship is between Xcode versions and clang versions, but the Xcode versions definitely do not imply the matching clang version.

MongoDB definitely does build with clang 9, but it is actually newer than our toolchain of record (clang 7), so we do no testing there either. We will probably upgrade to clang 9 or clang 10 as the minimum sometime during MongoDB 4.6 development.

Comment by Ralph Seichter [ 05/Feb/20 ]

Andrew, to make sure I understand this approach: Even though MongoDB 4.2.x can be successfully built using clang 9, which I believe to be bundled with Xcode 9, you are planning to change your build checks to actively block any Xcode version < 10.2 ? Why not just exclude the affected Xcode 10.1 ?

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

We will disable support for building with Xcode < 10.2, as it appears that Xcode 10.1 cannot actually build the v4.2 branch. It is not clear whether this is also true on the current master branch, as the code in question has been refactored there. However, it would be somewhat odd to have the master branch compiler minimum be lower than v4.2. So we will land this on master first and backport it to the v4.2 branch. The v4.0 branch only requires Xcode 8, so is not affected.

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

OK, I was able to log into one of our macOS 10.14 developer machines that fortunately has both Xcode 10.1 and Xcode 10.2 installed. I can reproduce the error when I build a slightly reduced version of that file with Xcode 10.1, but not with Xcode 10.2:

$ cat repro.cpp
/**
 *    Copyright (C) 2018-present MongoDB, Inc.
 *
 *    This program is free software: you can redistribute it and/or modify
 *    it under the terms of the Server Side Public License, version 1,
 *    as published by MongoDB, Inc.
 *
 *    This program is distributed in the hope that it will be useful,
 *    but WITHOUT ANY WARRANTY; without even the implied warranty of
 *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 *    Server Side Public License for more details.
 *
 *    You should have received a copy of the Server Side Public License
 *    along with this program. If not, see
 *    <http://www.mongodb.com/licensing/server-side-public-license>.
 *
 *    As a special exception, the copyright holders give permission to link the
 *    code of portions of this program with the OpenSSL library under certain
 *    conditions as described in each individual source file and distribute
 *    linked combinations including the program with the OpenSSL library. You
 *    must comply with the Server Side Public License in all respects for
 *    all of the code used other than as permitted herein. If you modify file(s)
 *    with this exception, you may extend this exception to your version of the
 *    file(s), but you are not obligated to do so. If you do not wish to do so,
 *    delete this exception statement from your version. If you delete this
 *    exception statement from all source files in the program, then also delete
 *    it in the license file.
 */
 
#pragma once
 
#include <cstdint>
 
#ifdef _MSC_VER
#include <SafeInt.hpp>
#endif
 
namespace mongo {
 
/**
 * Returns true if multiplying lhs by rhs would overflow. Otherwise, multiplies 64-bit signed
 * or unsigned integers lhs by rhs and stores the result in *product.
 */
constexpr bool mongoSignedMultiplyOverflow64(int64_t lhs, int64_t rhs, int64_t* product);
constexpr bool mongoUnsignedMultiplyOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* product);
 
/**
 * Returns true if adding lhs and rhs would overflow. Otherwise, adds 64-bit signed or unsigned
 * integers lhs and rhs and stores the result in *sum.
 */
constexpr bool mongoSignedAddOverflow64(int64_t lhs, int64_t rhs, int64_t* sum);
constexpr bool mongoUnsignedAddOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* sum);
 
/**
 * Returns true if subtracting rhs from lhs would overflow. Otherwise, subtracts 64-bit signed or
 * unsigned integers rhs from lhs and stores the result in *difference.
 */
constexpr bool mongoSignedSubtractOverflow64(int64_t lhs, int64_t rhs, int64_t* difference);
constexpr bool mongoUnsignedSubtractOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* difference);
 
 
#ifdef _MSC_VER
 
// The SafeInt functions return true on success, false on overflow.
 
constexpr bool mongoSignedMultiplyOverflow64(int64_t lhs, int64_t rhs, int64_t* product) {
    return !SafeMultiply(lhs, rhs, *product);
}
 
constexpr bool mongoUnsignedMultiplyOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* product) {
    return !SafeMultiply(lhs, rhs, *product);
}
 
constexpr bool mongoSignedAddOverflow64(int64_t lhs, int64_t rhs, int64_t* sum) {
    return !SafeAdd(lhs, rhs, *sum);
}
 
constexpr bool mongoUnsignedAddOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* sum) {
    return !SafeAdd(lhs, rhs, *sum);
}
 
constexpr bool mongoSignedSubtractOverflow64(int64_t lhs, int64_t rhs, int64_t* difference) {
    return !SafeSubtract(lhs, rhs, *difference);
}
 
constexpr bool mongoUnsignedSubtractOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* difference) {
    return !SafeSubtract(lhs, rhs, *difference);
}
 
#else
 
// On GCC and CLANG we can use __builtin functions to perform these calculations. These return true
// on overflow and false on success.
 
constexpr bool mongoSignedMultiplyOverflow64(long lhs, long rhs, long* product) {
    return __builtin_mul_overflow(lhs, rhs, product);
}
 
constexpr bool mongoSignedMultiplyOverflow64(long long lhs, long long rhs, long long* product) {
    return __builtin_mul_overflow(lhs, rhs, product);
}
 
constexpr bool mongoUnsignedMultiplyOverflow64(unsigned long lhs,
                                               unsigned long rhs,
                                               unsigned long* product) {
    return __builtin_mul_overflow(lhs, rhs, product);
}
 
constexpr bool mongoUnsignedMultiplyOverflow64(unsigned long long lhs,
                                               unsigned long long rhs,
                                               unsigned long long* product) {
    return __builtin_mul_overflow(lhs, rhs, product);
}
 
constexpr bool mongoSignedAddOverflow64(long lhs, long rhs, long* sum) {
    return __builtin_add_overflow(lhs, rhs, sum);
}
 
constexpr bool mongoSignedAddOverflow64(long long lhs, long long rhs, long long* sum) {
    return __builtin_add_overflow(lhs, rhs, sum);
}
 
constexpr bool mongoUnsignedAddOverflow64(unsigned long lhs,
                                          unsigned long rhs,
                                          unsigned long* sum) {
    return __builtin_add_overflow(lhs, rhs, sum);
}
 
constexpr bool mongoUnsignedAddOverflow64(unsigned long long lhs,
                                          unsigned long long rhs,
                                          unsigned long long* sum) {
    return __builtin_add_overflow(lhs, rhs, sum);
}
 
constexpr bool mongoSignedSubtractOverflow64(long lhs, long rhs, long* difference) {
    return __builtin_sub_overflow(lhs, rhs, difference);
}
 
constexpr bool mongoSignedSubtractOverflow64(long long lhs, long long rhs, long long* difference) {
    return __builtin_sub_overflow(lhs, rhs, difference);
}
 
constexpr bool mongoUnsignedSubtractOverflow64(unsigned long lhs,
                                               unsigned long rhs,
                                               unsigned long* sum) {
    return __builtin_sub_overflow(lhs, rhs, sum);
}
 
constexpr bool mongoUnsignedSubtractOverflow64(unsigned long long lhs,
                                               unsigned long long rhs,
                                               unsigned long long* sum) {
    return __builtin_sub_overflow(lhs, rhs, sum);
}
 
#endif
 
/**
 * Safe mod function which throws if the divisor is 0 and avoids potential overflow in cases where
 * the divisor is -1. If the absolute value of the divisor is 1, mod will always return 0. We fast-
 * path this to avoid the scenario where the dividend is the smallest negative long or int value and
 * the divisor is -1. Naively performing this % may result in an overflow when the -2^N value is
 * divided and becomes 2^N. See SERVER-43699.
 */
template <typename T>
constexpr T mongoSafeMod(T dividend, T divisor) {
    return (divisor == 1 || divisor == -1 ? 0 : dividend % divisor);
}
 
}  // namespace mongo

$ DEVELOPER_DIR=/Applications/Xcode10.1.0.app/ xcrun clang++ ./repro.cpp --std=c++17
./repro.cpp:30:9: warning: #pragma once in main file [-Wpragma-once-outside-header]
#pragma once
        ^
./repro.cpp:95:16: error: constexpr function never produces a constant expression [-Winvalid-constexpr]
constexpr bool mongoSignedMultiplyOverflow64(long lhs, long rhs, long* product) {
               ^
./repro.cpp:96:12: note: subexpression not valid in a constant expression
    return __builtin_mul_overflow(lhs, rhs, product);

However, Xcode 10.2 has no problem with it:

$ DEVELOPER_DIR=/Applications/Xcode10.2.0.app/ xcrun clang++ ./repro.cpp --std=c++17
./repro.cpp:30:9: warning: #pragma once in main file [-Wpragma-once-outside-header]
#pragma once
        ^
1 warning generated.
Undefined symbols for architecture x86_64:
  "_main", referenced from:
     implicit entry/start for main executable
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Xcode 11.3 is also happy with it:

$ DEVELOPER_DIR=/Applications/Xcode11.3.0.app/ xcrun clang++ ./repro.cpp --std=c++17
./repro.cpp:30:9: warning: #pragma once in main file [-Wpragma-once-outside-header]
#pragma once
        ^
1 warning generated.
Undefined symbols for architecture x86_64:
  "_main", referenced from:
     implicit entry/start for main executable
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

This suggests that the v4.2 codebase really requires Xcode 10.2, and we should upgrade our check for _apple_build_version_:

$ DEVELOPER_DIR=/Applications/Xcode10.2.0.app/ xcrun clang++ -dM -E -x c++ - < /dev/null | grep apple
#define __apple_build_version__ 10010046

However, the current SConscript checks look only to be at or newer than 10001044, which was presumably Xcode 10.0.0.

Unfortunately, if we make this change it does mean that MongoDB v4.2 will not be buildable with Xcode on macOS 10.13 or older, since Apple dropped 10.13 support in the minor version bump from Xcode 10.1 to 10.2. Builds for macOS 10.12 are still possible by building with the appropriate -mmacosx-version-min flag, but they will need to be run on a macOS 10.14+ machine using Xcode 10.2+.

Comment by Ryan Schmidt [ 05/Feb/20 ]

The reporter didn't say. The log attached to the MacPorts ticket says Xcode is not installed. (Presumably, only the command line tools are installed, or else MacPorts is unable to detect the Xcode version for some reason.) Xcode 10.1 is the last version that is compatible with High Sierra.

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

Do you know what minor and patch version Xcode is failing? Our macOS nightlies use Xcode 10.2.0 and we do not see this error.

Comment by Ryan Schmidt [ 05/Feb/20 ]

No, using the MacPorts clang 9 compiler with a working linker works, both on my system and on our buildbot.

Using Xcode 10's clang 1000.10.44.4 does not work, per the user's log. I don't believe questions about the linker are relevant here; the error comes from the compiler.

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

I don't think it is? There was one issue in the linked macports thing where the wrong linker was in play. That was resolved. Then there was an issue where if you didn't get Xcode as your compiler, it didn't work. But once you do get Xcode as your compiler, the problems with the overflow_arithmetic.h header go away. So I'm not sure what else there is to do?

Comment by Ryan Schmidt [ 05/Feb/20 ]

I don't understand how the linker is involved in the compiler generating a constexpr error message.

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

ryandesign - OK, so it sounds to me like everything works when the compiler and linker are the right versions? In that case, I'm inclined to close this ticket.

Comment by Ryan Schmidt [ 05/Feb/20 ]

The MacPorts clang that successfully built the port for me (on High Sierra with Xcode 9) was:

$ /opt/local/bin/clang-mp-9.0 -v
clang version 9.0.1
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /opt/local/libexec/llvm-9.0/bin

I'm the mongodb maintainer for MacPorts, so you can email me if there's anything I should know about, otherwise I'm sure I'll find out when I try to update our port to 4.4.

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

ryandesign - FYI, I tried building v4.2 HEAD with homebrew {{clang}}s of various versions newer than or equal to the required clang minimum of 7, and I was unable to reproduce those errors. I built as follows:

python buildscripts/scons.py --implicit-cache --build-fast-and-loose=on --dbg=on --opt=on --variables-files= --link-model=dynamic --install-mode=hygienic CC=/usr/local/opt/llvm@7/bin/clang CXX=/usr/local/opt/llvm@7/bin/clang++ CCFLAGS="-isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -mmacosx-version-min=10.12 -target darwin16.0.0 -arch x86_64" LINKFLAGS="-Wl,-syslibroot,/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -mmacosx-version-min=10.12 -target darwin16.0.0 -arch x86_64" build/optdebug/mongo/platform/overflow_arithmetic_test

Also tried with the llvm@8 and llvm@9 versions of the compiler, and the test always built OK.

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

ryandesign - Also, on a wholly different topic, what is the best way for me to flag upcoming build system changes to the macports developers responsible for MongoDB. We have some major build system interface changes coming in v4.4.

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

Another interesting data point: this file was substantially overhauled since v4.2:

I'm going to checkout v4.2 locally and build it with my newer (linux) clang and see if anything shakes out.

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

Oh that is interesting. It suggests that this newer clang is seeing something that is perhaps really erroneous in our code and flagging it. Do you happen to know what version of clang is actually getting used?

Comment by Ryan Schmidt [ 05/Feb/20 ]

Ah! I forgot. We are not disabling minimum compiler version enforcement. We are blacklisting Xcode clangs less than version 1000, so with Xcode 9, a newer MacPorts clang would indeed be used.

Comment by Andrew Morrow (Inactive) [ 05/Feb/20 ]

ryandesign -

The MongoDB build system should, on the v4.2 branch, be requiring Xcode 10 or newer: https://github.com/mongodb/mongo/blob/68ed6c33387eb9111568d9ebd7d20fdd3dfc1530/SConstruct#L2039-L2058, so I'm confused how it could be building with Xcode 9, unless our check is erroneous. Or are you building with --disable-minimum-comiler-version-enforcement?

Comment by Ryan Schmidt [ 05/Feb/20 ]

Just to clarify: from the log attached to the MacPorts ticket, it is confirmed that the user is using macOS 10.13 (which is High Sierra), not macOS 10.14 (which is Mojave). The user is also using Xcode 10. This version of mongodb built successfully on macOS 10.13 (High Sierra) on my machine and on the MacPorts automated build system, both using Xcode 9.

Generated at Thu Feb 08 05:10:03 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.