Eric Fiselier via llvm-dev
2019-Mar-08 19:38 UTC
[llvm-dev] 8.0 Regression with __builtin_constant_p.
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 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190308/c26c3f4c/attachment.html>
Bill Wendling via llvm-dev
2019-Mar-08 22:45 UTC
[llvm-dev] [cfe-dev] 8.0 Regression with __builtin_constant_p.
Hi Eric, The attached patch may help matters, though it now fails because "test2" doesn't exist (it's not enabled). -bw On Fri, Mar 8, 2019 at 11:38 AM Eric Fiselier via cfe-dev < cfe-dev at lists.llvm.org> wrote:> 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 > _______________________________________________ > cfe-dev mailing list > cfe-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190308/30c031a7/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: bcp.patch Type: text/x-patch Size: 624 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190308/30c031a7/attachment.bin>
Eric Fiselier via llvm-dev
2019-Mar-08 22:57 UTC
[llvm-dev] [cfe-dev] 8.0 Regression with __builtin_constant_p.
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 On Fri, Mar 8, 2019 at 5:46 PM Bill Wendling <isanbard at gmail.com> wrote:> Hi Eric, > > The attached patch may help matters, though it now fails because "test2" > doesn't exist (it's not enabled). > > -bw > > > > On Fri, Mar 8, 2019 at 11:38 AM Eric Fiselier via cfe-dev < > cfe-dev at lists.llvm.org> wrote: > >> 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 >> _______________________________________________ >> cfe-dev mailing list >> cfe-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190308/1efab7c5/attachment.html>