[libc-commits] [PATCH] D119145: [libc] Fix illegal type punning

Guillaume Chatelet via Phabricator via libc-commits libc-commits at lists.llvm.org
Tue Feb 8 02:33:31 PST 2022

gchatelet added inline comments.

Comment at: libc/src/__support/CPP/Bit.h:35
+  char *dst = reinterpret_cast<char *>(&to);
+  const char* src=reinterpret_cast<const char *>(&from));
lntue wrote:
> Extra space around `=`, and extra closing parenthesis to be removed.
Duh!? What happened here...

Comment at: libc/src/__support/CPP/Bit.h:37-39
+  __builtin_memcpy_inline(dst, src, SIZE);
+  for (unsigned i = 0; i < SIZE; ++i)
lntue wrote:
> Is SIZE defined anywhere?
Bad copy/paste, thx for noticing.

Comment at: libc/src/fenv/fesetexceptflag.cpp:28
                 "fexcept_t value cannot fit in an int value.");
-  int excepts_to_set = static_cast<const int>(*flagp) & excepts;
+  using IntType = TypeSelector<sizeof(fexcept_t)>::type;
+  const int excepts_as_int = __llvm_libc::bit_cast<IntType>(*flagp);
sivachandra wrote:
> I am still confused. Previously, we de-referenced (`*flagp`) before static-casting it. So, there is no type-punning involved (assuming `fexcept_t` is of integral type), no? Can you share the error you are seeing?
Oh I see, there is no type punning here indeed. I got confused by using `bit_cast` that expects both type to have exact same size.

That said this is still //implementation defined// because of [[ https://en.cppreference.com/w/c/language/conversion#:~:text=otherwise%2C%20if%20the%20target%20type%20is%20signed%2C%20the%20behavior%20is%20implementation%2Ddefined%20(which%20may%20include%20raising%20a%20signal) | integer conversion from unsigned to signed ]].

This currently works for GCC and Clang but the standard does not enforce this behavior [[ https://en.cppreference.com/w/cpp/20#:~:text=signed%20integers%20are%202%27s%20complement | before c++20 ]].

  rG LLVM Github Monorepo



More information about the libc-commits mailing list