Ian RogersMar 21
Ugh.. when people won't take fixes because in their opinion the C specification is wrong and their feeling about how compilers work should be how it works:
https://lore.kernel.org/lkml/CAADnVQJN[email protected]/
Re: [PATCH v1 1/2] libbpf: Fix strict aliasing violations in hashmap - Alexei Starovoitov

Show threadAiden Grossman

@irogers Yeah, this is pretty frustrating.

I remember seeing a lot of this when looking at where people where people were passing -fno-lifetime-dse across OSS projects when implementing the optimization in clang. There were several comments complaining about compiler optimizations, with optimizations quoted for emphasis.

I guess the behavior of optimizations like -flifetime-dse and -fstrict-aliasing can be somewhat surprising for users. But the C/C++ standards are what they are and these rules do help the compiler optimize better.

Mar 22 at 6:06pmWeb
Show threadIan RogersMar 22
@boomanaiden154 when we transitioned Android to clang a common practice in Android was to have functions doing "this == NULL" particularly in helper cast functions, which would then just return NULL. Clang/llvm would remove these tests as "this" can never be NULL causing chaos. The reaction from senior engineers was clang was wrong and we needed our own C++ specification. 🤷
Show threadAndrea (Drea) Tamar Pinski6d ago

@boomanaiden154 @irogers

Firefox and LLVM were the two places I know of which had to use -flifetime-dse=0.
I know when LLVM implemented the optimization, LLVM had to finally fix the issue (GCC folks had reported it to both LLVM and firefox over 10 years ago even). It is funny that it was LLVM folks who forced the fix.

-fsanitize=address does detect most of the issue though which is a good thing.

Show threadAiden Grossman6d ago

@pinskia @irogers

Yeah, we did get to fixing `-flifetime-dse` in our own code pretty late. It only broke when building LLVM with gcc with LTO and the fix was relatively tricky, so not a lot of motivation for people to fix it.

It's interesting that Firefox still passes it. Chromium landed a couple of related fixes when it landed in LLVM, but I think the most interesting find there was that some of the tests were running without msan (since fixed) which is where the only real issue was from what I've read. They still have it turned off due to the issues it has caused and there being no noticeable performance impact for them/little size impact (I think a hundredth of a percent or something).