Illegal instruction because of mixing generic code and AVX code

[Luboš Luňák]

Hello,

it appears that CPU-specific sources such as SkOpts_avx.cpp pull in generic
code that may cause illegal instruction crashes.

We've had a crash reported[*] saying that SkRect::round() causes an illegal
instruction crash. We build Skia sources targetting only SSE2, but
disassembling the binary showed AVX instructions in SkRect::round(). A
possible explanation for what happens is that SkOpts_avx.cpp pulls in
SkRect.h header (SkOpts_avx.cpp -> SkOpts.h -> SkRasterPipeline.h ->
SkImageInfo.h -> SkRect.h), SkRect::round() is considered too large for
inlining so a weak copy is emitted built with AVX, and when the linker merges
weak symbols it happens to select the AVX-built copy as the one non-inline
copy of SkRect::round(), causing AVX code to be called from generic SSE2-only
code.

Am I getting it right that code such as SkOpts_avx.cpp should not pull in
such generic code and that it is a bug that this happens for SkRect?

[*] https://bugs.documentfoundation.org/show_bug.cgi?id=144598#c15

--
Luboš Luňák
l.l...@collabora.com

[Luboš Luňák]

On Monday 20 of September 2021, Luboš Luňák wrote:
> Hello,
>
> it appears that CPU-specific sources such as SkOpts_avx.cpp pull in
> generic code that may cause illegal instruction crashes.

...

> Am I getting it right that code such as SkOpts_avx.cpp should not pull in
> such generic code and that it is a bug that this happens for SkRect?

It turned out the problem was that cl.exe (and thus also clang-cl) likes to
emit copies of inline functions even when not necessary (for the /Zc:inline-
misfeature, apparently). So SkOpts_avx.o had copies of a number of inline
functions and in some builds the linker decided to keep those when merging.
Using clang-cl's /Zc:dllexportInlines- appears to avoid or at least
significantly reduce this problem.

--
Luboš Luňák
l.l...@collabora.com