google-cloud-cpp icon indicating copy to clipboard operation
google-cloud-cpp copied to clipboard

ci: upgrade LLVM to 18

Open alevenberg opened this issue 1 year ago • 6 comments

https://github.com/googleapis/google-cloud-cpp/issues/14076

This change is Reviewable

alevenberg avatar May 07 '24 13:05 alevenberg

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.23%. Comparing base (10e7751) to head (1c8672a). Report is 43 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14148      +/-   ##
==========================================
- Coverage   93.79%   93.23%   -0.57%     
==========================================
  Files        2293     2206      -87     
  Lines      202903   192133   -10770     
==========================================
- Hits       190317   179135   -11182     
- Misses      12586    12998     +412     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 07 '24 13:05 codecov[bot]

https://releases.llvm.org/18.1.0/projects/libcxx/docs/ReleaseNotes.html#build-system-changes

This might be relevant

When the shared/static library is built with -fno-exceptions, the behavior of operator new was changed to make it standards-conforming. In LLVM 17 and before, the throwing versions of operator new would return nullptr upon failure to allocate, when the shared/static library was built with exceptions disabled. This was non-conforming, since the throwing versions of operator new are never expected to return nullptr, and this non-conformance could actually lead to miscompiles in subtle cases.

Starting in LLVM 18, the throwing versions of operator new will abort the program when they fail to allocate if the shared/static library has been built with -fno-exceptions. This is consistent with the behavior of all other potentially-throwing functions in the library, which abort the program instead of throwing when -fno-exceptions is used.

Furthermore, when the shared/static library is built with -fno-exceptions, users who override the throwing version of operator new will now need to also override the std::nothrow_t version of operator new if they want to use it. Indeed, this is because there is no way to implement a conforming operator new(nothrow) from a conforming potentially-throwing operator new when compiled with -fno-exceptions. In that case, using operator new(nothrow) without overriding it explicitly but after overriding the throwing operator new will result in an error.

Note that this change only impacts vendors/users that build the shared/static library themselves and pass -DLIBCXX_ENABLE_EXCEPTIONS=OFF, which is not the default configuration. If you are using the default configuration of the library, the libc++ shared/static library will be built with exceptions enabled, and there is no change between LLVM 17 and LLVM 18, even for users who build their own code using -fno-exceptions.

It seems like the failures have to do with throwing exceptions

alevenberg avatar May 10 '24 13:05 alevenberg

Running locally, getting things like you can't use try or throw when exceptions are disabled

In file included from ./generator/internal/codegen_utils.h:18:
./generator/internal/printer.h:79:5: error: cannot use 'throw' with exceptions disabled
   79 |     throw std::runtime_error(
      |     ^
./generator/internal/printer.h:76:39: error: cannot use 'try' with exceptions disabled
   76 |              std::string const& text) try {
      |                                       ^
./generator/internal/printer.h:93:5: error: cannot use 'throw' with exceptions disabled
   93 |     throw std::runtime_error(
      |     ^
./generator/internal/printer.h:90:30: error: cannot use 'try' with exceptions disabled
   90 |              Args&&... args) try {
      |                              ^

alevenberg avatar May 10 '24 14:05 alevenberg

https://releases.llvm.org/18.1.0/projects/libcxx/docs/ReleaseNotes.html#build-system-changes

This might be relevant

I am not sure how that would affect the msan build. We only have one build that compiles with -fno-exceptions, that is noex. The msan build compiles with exceptions enabled.

It seems like the failures have to do with throwing exceptions

I suggested turning exceptions off as a way to verify that hypothesis. When we turn exceptions off, nothing throws (we get more crashes). If that fixes the problem then there is something weird with exceptions.

coryan avatar May 10 '24 14:05 coryan

Running locally, getting things like you can't use try or throw when exceptions are disabled

Blegh. Can we skip the generator just to test things? As in bazel test //google/cloud/... vs. bazel test //.... It is possible (but not certain) that ci/etc/cloudcxxrc can be used to configure that.

coryan avatar May 10 '24 14:05 coryan

Running locally, getting things like you can't use try or throw when exceptions are disabled

Blegh. Can we skip the generator just to test things? As in bazel test //google/cloud/... vs. bazel test //.... It is possible (but not certain) that ci/etc/cloudcxxrc can be used to configure that.

It seems like we also use try in our quickstarts

alevenberg avatar May 10 '24 14:05 alevenberg

FTR, it was related to exceptions: by default libc++ now compiles with the LLVM implementation of stack unwinding under exceptions. Meanwhile, the compiler on Fedora is (naturally) trying to use the "native" implementation of stack unwinding. The two things do not work together.

coryan avatar May 14 '24 18:05 coryan