Update Dockerfile flags #625
Conversation
| --extra-cxxflags="$CXXFLAGS" \ | ||
| --extra-ldexeflags="-fPIE -static-pie" \ | ||
| --extra-ldflags="-fopenmp -Wl,--allow-multiple-definition -Wl,-z,stack-size=2097152" \ | ||
| --toolchain=hardened \ |
There was a problem hiding this comment.
Wonder if we should keep this if ffmpeg starts to use addition flags?
There was a problem hiding this comment.
well, looking at the configure for FFmpeg it seems like it was not valuable when we added our needed cflags manually.
There was a problem hiding this comment.
thats the only thing I removed it based on. previously, it would only work with hardened toolchain flag, but since it now works with the right cflags without hardened toolchain, I didn't want it messing up the cflags that worked. I can't remember if I tested it with hardened toolchain or not. I just know it worked without it.
There was a problem hiding this comment.
huh, failed with hardened flag when building for armv7 or what was the build env? remember what/how it failed?
There was a problem hiding this comment.
Yeah this doesn't enable armv7, I wasn't able to get that working. I basically gave up on that, but this change came from that debugging. So I thought I would add this anyways
There was a problem hiding this comment.
Sorry i'm still a bit confused what kind of build your trying to enable? i think it would be good to keep the hardened flag to ffmpeg configure if they ever add more flags or if they start using it to make ffmpeg's own source more fortified etc
There was a problem hiding this comment.
Nothing changed in behavior other than simplifying the build steps without the hack.
There was a problem hiding this comment.
Still a bit confused :) so is it that --toolchain=hardened adds -pie and that does not play nice with --extra-ldexeflags="-fPIE -static-pie"?
There was a problem hiding this comment.
yeah, I assumed that's the reason you did the sed/hardened. Because it does -pie and that conflicts with -static-pie.
--toolchain=hardened also does:
add_cppflags -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
add_cflags -fno-strict-overflow -fstack-protector-all
which we could add explcitly if they are needed. But I'm not sure what those values are or how they are used.
There was a problem hiding this comment.
Aha no i for some reason didn't see that --extra-ldexeflags existed so i used sed instead.
FORTIFY_SOURCE is a libc thing (possible other libs also?), to add additional checks etc. Now i actually checked and it seems musl is not using it but might do in the future.
-fno-strict-overflow -fstack-protector-all tells to compiler to generate code with various safe guards.
And i guess that the ffmpeg devs might add more or these in the future so it would be nice to automatically get them. Other option could be to from time to time sync the Dockerfile flags with what ffmpeg uses.
removes sed and toolchain hack with regular cflags.