biojppm/rapidyaml

ODR violation when compiling with LTO/C++17

Closed this issue · 5 comments

biojppm commented 
          So I started by trying to imitate Fedora’s build flags as closely as I reasonably could, with the idea that if I could reproduce this with `samples/add_subdirectory`, I would 

$ mock -r fedora-rawhide-ppc64le --clean
$ mock -r fedora-rawhide-ppc64le -i git-core cmake gcc-c++ 'cmake(gtest)'
$ mock -r fedora-rawhide-ppc64le --shell --enable-network
<mock-chroot> sh-5.2# git clone https://github.com/biojppm/rapidyaml.git
<mock-chroot> sh-5.2# cd rapidyaml/
<mock-chroot> sh-5.2# git checkout fix/464_ub_ppc64le
<mock-chroot> sh-5.2# git submodule update --init --recursive
<mock-chroot> sh-5.2# cd samples/add_subdirectory
<mock-chroot> sh-5.2# CXXFLAGS="-O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -mcpu=power8 -mtune=power8 -fasynchronous-unwind-tables -fstack-clash-protection" cmake -S . -B build -DRYML_CXX_STANDARD=14
<mock-chroot> sh-5.2# cmake --build build -j16 --target run
[ 95%] Linking CXX executable ryml-quickstart
/builddir/rapidyaml/ext/c4core/src/c4/charconv.hpp:197:3: warning: type ‘c4::RealFormat_e’ violates the C++ One Definition Rule [-Wodr]
  197 | } RealFormat_e;
      |   ^
/builddir/rapidyaml/ext/c4core/src/c4/charconv.hpp:209:3: note: an enum with different value name is defined in another translation unit
  209 | } RealFormat_e;
      |   ^
/builddir/rapidyaml/ext/c4core/src/c4/charconv.hpp:190:5: note: name ‘FTOA_FLOAT’ is defined as 32-bit while another translation unit defines it as 8-bit
  190 |     FTOA_FLOAT = static_cast<std::underlying_type<std::chars_format>::type>(std::chars_format::fixed),
      |     ^
/builddir/rapidyaml/ext/c4core/src/c4/charconv.hpp:202:5: note: mismatching definition
  202 |     FTOA_FLOAT = 'f',
      |     ^
[ 95%] Built target ryml-quickstart
[100%] running: /builddir/rapidyaml/samples/add_subdirectory/build/ryml-quickstart
WTF1:
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
gmake[3]: *** [CMakeFiles/run.dir/build.make:71: CMakeFiles/run] Segmentation fault (core dumped)

I haven’t had a chance to try to minimize the compiler flags yet, but that looked interesting enough to go ahead and report it!

Originally posted by @musicinmybrain in #464 (comment)

biojppm commented

@musicinmybrain I think this is happening because the sample does not use RYML_CXX_STANDARD in its cmake file. The tell-tale sign is that if you use --std=c++17 or whatever, things work as expected.

That is, with RYML_CXX_STANDARD the library ends up being compiled with a different standard than the application.

So the solution is simple: not to use RYML_CXX_STANDARD.

biojppm commented

In other words, this does not seem to be an issue.

musicinmybrain commented

I definitely agree with your assessment of the reason for this, and that the experiments in #464 showed that it’s unrelated to any test failures.

We do have some mixing of C++ versions in Fedora:

  • the c4core package is built as C++11 because that’s its default
  • rapidyaml is C++14 (via RYML_CXX_STANDARD) because it uses the system gtest for testing, and gtest 1.13.0 and later require at least C++14
  • jsonnet is implicitly C++17 because that’s the current GCC default

In almost all cases, we are able to link together shared libraries built with different C++ standards without problems. This isn’t necessarily true across all C++ compilers, but it’s true when everything is built with GCC, because GCC specifically tries to guarantee that. There are a few cases where a design decision can subvert this – like this case, where the preprocessor is used to give a type has a different layout depending on the standard version. Even there, we don’t encounter trouble in practice because c4project and rapidyaml are both compiled as shared libraries, and c4::RealFormat_e doesn’t appear in the signatures of any externally visible functions. (We can verify this with e.g. objdump -C -T usr/lib64/libc4core.so.0.2.1 and objdump -C -T usr/lib64/libryml.so.0.7.1.)

So the use of shared libraries protects us from any ODR issues in practice; they appeared in #464 because I was working in a git checkout and linking statically. I agree that “just don’t do that” is a reasonable thing to say about that.

I could preemptively start build c4core and rapidyaml as C++17, but it doesn’t really seem to matter one way or the other.

biojppm commented

Well, the recommendation is of course that if c4core is linked to ryml, then both should use the same standard. In this case, I would recommend bringing up Fedora's c4core to c++14 (although I don't know if that's up to you).

But anyway, this is a user problem. So I'll close this issue for now.

musicinmybrain commented

Well, the recommendation is of course that if c4core is linked to ryml, then both should use the same standard. In this case, I would recommend bringing up Fedora's c4core to c++14 (although I don't know if that's up to you).

It is, and since we generally defer to upstream defaults when choosing a C++ standard, I’ll cite this comment when I do it.

  • 👍1