intel / libyami

Yet Another Media Infrastructure. it is core part of media codec with hardware acceleration, it is yummy to your video experience on Linux like platform.
Apache License 2.0
146 stars 106 forks source link

Add flags for SDL signoff called SDL325 - Compile With Defenses Enabled #859

Closed wangzj0601 closed 6 years ago

wangzj0601 commented 6 years ago

Due to the flags SDL required for Linux again, we need add these flags for SDL signoff called SDL325 - Compile With Defenses Enabled

uartie commented 6 years ago

It's looking better. But it appears someone forgot to add $(AM_CXXFLAGS) to the libgtest_la_CXXFLAGS target in gtestsrc/Makefile.am... can you add that, please, to silence the new warning about "FORTIFY_SOURCE requires -O" I'm seeing now?

Also, while at it, please use same format as we did for AM_CFLAGS substitution in configure.ac (i.e. use a one-liner AC_SUBST).

wangzj0601 commented 6 years ago

@uartie, the option "FORTIFY_SOURCE" does need "-O", thank you for your reply over and over again very much

xuguangxin commented 6 years ago

@uartie ,thanks for reivew. @wangzj0601 , why we need -O 2 bot in CPP_FLAGS AND CXX_FLAGS. I remember CPP is for some process, not compile flag, why it needs -O 2.

wangzj0601 commented 6 years ago

This -O should be for "-D_FORTIFY_SOURCE=2", only -O is set to 2 or higher, then the option "FORTIFY_SOURCE" can be activated.

xuguangxin commented 6 years ago

will this –O2 covered by CPP_FLAGS? If someone need define –O 0 for debug. Will this build failed?

thanks

From: wangzj0601 [mailto:notifications@github.com] Sent: Friday, June 15, 2018 2:17 PM To: intel/libyami libyami@noreply.github.com Cc: Xu, Guangxin guangxin.xu@intel.com; Mention mention@noreply.github.com Subject: Re: [intel/libyami] Add flags for SDL signoff called SDL325 - Compile With Defenses Enabled (#859)

This -O should be for "-D_FORTIFY_SOURCE=2", only -O is set to 2 or higher, then the option "FORTIFY_SOURCE" can be activated.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/intel/libyami/pull/859#issuecomment-397524598, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AGW9GmXiopkMmP3N8y-mijHyi5gHCHNkks5t81FtgaJpZM4Ul7jF.

wangzj0601 commented 6 years ago

This -O2 and CPPFLAGS will be compiled together. For debug, you can modify the makefile and change O2 to O0, it will not build failed.

uartie commented 6 years ago

CXXFLAGS get applied to c++ targets. CFLAGS get applied to c targets. CPPFLAGS get applied to both c and c++ targets. Thus, -O in all of these is redundant.

uartie commented 6 years ago

The implicit compiler command in make for c targets is essentially $(CC) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) .... Likewise, for c++ targets. For the compiler, the last option seen overrides any previous option. Therefore, if user wants to use -O0 instead of -O2, they can set user CFLAGS="-O0" and CXXFLAGS="-O0" in environment to override the project AM_ flags. There is no need for user to modify the makefile or configure.ac by hand.

uartie commented 6 years ago

One more lesson ;-) ...

Technically, CPPFLAGS are for preprocessor and CFLAGS/CXXFLAGS are for compiler. But in terms of the way make assembles the final command, it doesn't make much difference (mostly).

uartie commented 6 years ago

...the caveat: CFLAGS/CXXFLAGS get applied by make during link phase, too. But, the CPPFLAGS do not get applied by make during link phase. Therefore, it would be better to put flags into the proper places. (see https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html for valid preprocessor flags)

uartie commented 6 years ago

LGTM