search

google / oss-fuzz

OSS-Fuzz - continuous fuzzing for open source software.
https://google.github.io/oss-fuzz
Apache License 2.0
10.39k stars 2.2k forks source link

mpv: fix introspector build #12081

Closed kasper93 closed 3 months ago

kasper93 commented 3 months ago

Workarounds the issue where compile tests would fail with -Werror=ignored-optimization-argument because Meson doesn't allow linker flags in CFLAGS or CXXFLAGS.

See: https://github.com/mesonbuild/meson/issues/6377#issuecomment-575977919

Thanks to @evverx for the idea: https://github.com/google/oss-fuzz/pull/7583#issuecomment-1104011067

This is a fragile workaround, but it looks like there isn't much else we can do.

github-actions[bot] commented 3 months ago

kasper93 is either the primary contact or is in the CCs list of projects/mpv.
kasper93 has previously contributed to projects/mpv. The previous PR was #12019

kasper93 commented 3 months ago

By the way, have you considered using LLD with thin lto? It would be much faster, current build is quite resource heavy.

jonathanmetzman commented 3 months ago

Hmmm...I guess we probably should be doing this (though I've vaguely heard that moLD is the best right now!). Linking is blackmagic to me. I'm confused though, why are you using gold but suggesting lld?

kasper93 commented 3 months ago

I'm confused though, why are you using gold but suggesting lld?

I use gold, because this is what is requested in default CFLAGS. I don't want to change default flags, this change is only to force Meson to do the correct thing.

jonathanmetzman commented 3 months ago

I'm confused though, why are you using gold but suggesting lld?

I use gold, because this is what is requested in default CFLAGS. I don't want to change default flags, this change is only to force Meson to do the correct thing.

Ah introspector asks for this. :-(

DavidKorczynski commented 3 months ago

We need full LTO for introspector though -- @kasper93 do you know if lld supports full lto?

kasper93 commented 3 months ago

do you know if lld supports full lto?

Yes, of course. In fact in both cases the actual LTO is handled by the same libLTO https://github.com/llvm/llvm-project/tree/main/llvm/lib/LTO For gold it is loaded as a LTO plugin. For LLD it is directly integrated inside the linker. But the main code is the same in both cases. I don't know what are the actual differences in cases like linking also with "non-lto" objects. Generally, nowadays LLD is good default option and since the builds are already bases heavily on LLVM it could be done for linking too. Though without thin lto, there would be no difference in performance actually.