8.0 Regression with __builtin_constant_p.

Project Infrastructure LLVM Dev List Archives
1

Hi All,

I know we’re late in the release process, but I’ve discovered a regression I believe is serious enough to hold the release [1].

The regression is in usages of __builtin_constant_p applied to a dereferenced pointer in a constant expression. In certain cases Clang now rejects this as a non-constant expression [2].

This regression will have a larger impact than it initially appears.

libstdc++ 7.1 and newer use __builtin_constant_p to implement C++17 std::char_traits [4]. This means that any usage of std::string_view in a constant expression has the possibility to break. __builtin_constant_p may be rare, but std::string_view is not.

This regression was first found because it breaks absl::StrFormat [4]. This bug will break Abseil’s LTS w/ libstdc++. Abseil cannot fully work around this bug.

I would implore us to fix this bug in the upcoming 8 release.

/Eric

[1] https://reviews.llvm.org/D59038
[2] https://godbolt.org/z/VOCCJF

[3] https://github.com/abseil/abseil-cpp/issues/271

[4] https://github.com/gcc-mirror/gcc/blob/gcc-8_1_0-release/libstdc+±v3/include/bits/char_traits.h#L229

2

Hi Eric,

The attached patch may help matters, though it now fails because “test2” doesn’t exist (it’s not enabled).

-bw

bcp.patch (624 Bytes)

3

Hi Bill,

I committed a fix already as r355743 [1], and it fixes test2 as well.

I believe we should merge this into 8.0.

/Eric

[1] https://github.com/llvm/llvm-project/commit/680e865c313a80b6ec329abde61e1f0c66bdc103

4

Ah! Cool. I’ll let the release manager make that decision. :slight_smile:

5

Okay, sounds like this is important enough, and now that there's a fix
committed I'll make sure it gets into the release. Let's have it bake
in trunk a little bit first though.