From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39349) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwsKO-0003PV-Nl for qemu-devel@nongnu.org; Thu, 12 Nov 2015 08:54:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZwsKN-0005pP-Hh for qemu-devel@nongnu.org; Thu, 12 Nov 2015 08:54:16 -0500 MIME-Version: 1.0 In-Reply-To: <87bnazmjlc.fsf@blackfin.pond.sub.org> References: <1447275457-1415-1-git-send-email-rprebello@gmail.com> <8737wbvcc2.fsf@blackfin.pond.sub.org> <87bnazmjlc.fsf@blackfin.pond.sub.org> Date: Thu, 12 Nov 2015 11:54:12 -0200 Message-ID: From: Rodrigo Rebello Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 1/1] configure: use appropriate code fragment for -fstack-protector checks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-trivial@nongnu.org, qemu-devel@nongnu.org Markus, 2015-11-12 11:29 GMT-02:00 Markus Armbruster : > Rodrigo Rebello writes: > >> Markus, >> >> 2015-11-12 6:41 GMT-02:00 Markus Armbruster : >>> Rodrigo Rebello writes: >>> >>>> The check for stack-protector support consisted in compiling and linking >>>> the test program below (output by function write_c_skeleton()) with the >>>> compiler flag -fstack-protector-strong first and then with >>>> -fstack-protector-all if the first one failed to work: >>>> >>>> int main(void) { return 0; } >>>> >>>> This caused false positives when using certain toolchains in which the >>>> compiler accepted -fstack-protector-strong but no support was provided >>>> by the C library, since for this stack-protector variant the compiler >>>> emits canary code only for functions that meet specific conditions >>>> (local arrays, memory references to local variables, etc.) and the code >>>> fragment under test included none of them (hence no stack protection >>>> code generated, no link failure). >>>> >>>> This fix changes the test program used for -fstack-protector checks to >>>> include a function that meets conditions which cause the compiler to >>>> generate canary code in all variants. >>>> >>>> Signed-off-by: Rodrigo Rebello >>>> --- >>>> configure | 18 ++++++++++++++++++ >>>> 1 file changed, 18 insertions(+) >>>> >>>> diff --git a/configure b/configure >>>> index 46fd8bd..c3d9592 100755 >>>> --- a/configure >>>> +++ b/configure >>>> @@ -1486,6 +1486,24 @@ for flag in $gcc_flags; do >>>> done >>>> >>>> if test "$stack_protector" != "no"; then >>>> + cat > $TMPC << EOF >>>> +void foo(const char *c); >>>> + >>>> +void foo(const char *c) >>>> +{ >>>> + char arr[64], *p; >>>> + for (p = arr; *c; c++, p++) { >>>> + *p = *c; >>>> + } >>>> +} >>>> + >>>> +int main(void) >>>> +{ >>>> + char c[] = ""; >>>> + foo(c); >>> >>> Why not simply foo("")? >>> >>> Could the optimizer optimize away the pattern that triggers the canary? >>> >>> To protect against that possibility, we could use >>> >>> int main(int argc, char *argv[]) >>> { >>> foo(argv[0]); >>> } >>> >> >> You're right, this can be made simpler and the version you suggested >> works as well (even if I force different optimization levels in >> QEMU_CFLAGS). >> In fact, I've come up with an even simpler version which does not >> involve a "foo" function: >> >> int main(int argc, char *argv[]) >> { >> char arr[64], *p = arr, *c = argv[0]; >> while (*c) { >> *p++ = *c++; >> } >> return 0; >> } >> >> What do you think of this one? > > There's the theoretical possibility that the compiler treats main() > specially. > > But then there's also the even more theoretical possibility that the > compiler inlines foo() into main() at link time, throws away foo(), and > treats main() specially. > > We can worry about theoretical possibilities all day long. Instead, > please use your judgement to pick something that works now and looks > reasonably robust to you. Ok, then. I'll stick to the last version I proposed and send a new patch. Regards, Rodrigo