From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2653 invoked by alias); 3 Jan 2014 21:43:38 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 2643 invoked by uid 89); 3 Jan 2014 21:43:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 03 Jan 2014 21:43:36 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s03LhZok014216 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 3 Jan 2014 16:43:35 -0500 Received: from oldenburg.str.redhat.com (ovpn-116-33.ams2.redhat.com [10.36.116.33]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s03LhXvS030055 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 3 Jan 2014 16:43:34 -0500 Message-ID: <52C72F05.2060901@redhat.com> Date: Fri, 03 Jan 2014 21:43:00 -0000 From: Florian Weimer User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Jakub Jelinek CC: GCC Patches , shenhan@google.com Subject: Re: Extend -fstack-protector-strong to cover calls with return slot References: <52C6B807.1070203@redhat.com> <20140103185715.GO892@tucnak.redhat.com> In-Reply-To: <20140103185715.GO892@tucnak.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-01/txt/msg00132.txt.bz2 On 01/03/2014 07:57 PM, Jakub Jelinek wrote: >> +/* Check if the basic block has a call which uses a return slot. */ >> + >> +static bool >> +call_with_return_slot_opt_p (basic_block bb) >> +{ >> + for (gimple_stmt_iterator gsi = gsi_start_bb (bb); >> + !gsi_end_p (gsi); gsi_next (&gsi)) >> + { >> + gimple stmt = gsi_stmt (gsi); >> + if (gimple_code (stmt) == GIMPLE_CALL > > That would be is_gimple_call (stmt) instead. Ah, it's not used consistently everywhere, and I got it from of the leftover places. > Also, I'd think the function is misnamed, given that it checks if there > are any calls with return_slot_opt_p in a bb. I think it would be > better to move FOR_ALL_BB_FN (bb, cfun) loop also into the > function and call it any_call_... I should probably move both loops (the one for declarations and the one for basic blocks) into its own function. > Lastly, I wonder if gimple_call_return_slot_opt_p is really what you are > after, why does NRV matter here? The C code we generate does not construct the returned value in place (presumably because the partial write would be visible with threads, longjmp etc.), unlike the C++ code. That's why I'm interested in instrumenting NRV-able calls only. But gimple_call_return_slot_opt_p doesn't actually give me that. The GIMPLE from the C test case looks like this (before and after applying your proposal): foo11 () { int D.2265; struct B D.2266; D.2266 = global3 (); [return slot optimization] D.2265 = D.2266.a1; return D.2265; } In both cases, SSP instrumentation is applied: .type foo11, @function foo11: .LFB24: .cfi_startproc subq $56, %rsp .cfi_def_cfa_offset 64 movq %rsp, %rdi movq %fs:40, %rax movq %rax, 40(%rsp) xorl %eax, %eax call global3 movq 40(%rsp), %rdx xorq %fs:40, %rdx movl (%rsp), %eax jne .L50 addq $56, %rsp .cfi_remember_state .cfi_def_cfa_offset 8 ret .L50: .cfi_restore_state call __stack_chk_fail .cfi_endproc > Isn't what you are looking for instead > whether the called function returns value through invisible reference, > because then you'll always have some (aggregate) addressable object > in the caller's frame and supposedly you are after making sure that the > callee doesn't overflow the return object. > > So, looking at tree-nrv.c, that check would be roughly: > if (is_gimple_call (stmt) > && gimple_call_lhs (stmt) > && aggregate_value_p (TREE_TYPE (gimple_call_lhs (stmt)), > gimple_call_fndecl (stmt))) When I do that, I get SSP instrumentation even when the struct is small enough to be returned in registers. gimple_call_return_slot_opt_p returns false in this case. So gimple_call_return_slot_opt_p appears to be misnamed (it's an ABI thing, not really an optimization), but it's closer to what I want. -- Florian Weimer / Red Hat Product Security Team