From patchwork Mon Jun 4 17:54:57 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 162846 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id CFD94B6EEC for ; Tue, 5 Jun 2012 03:55:42 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1339437343; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject: References:In-Reply-To:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=dAZMXiBFB/rP2Vpo+KMtpJ8iaDs=; b=UsEezuFPQF+PMb/ kQnn2mlEayaKtnpqlB4/kO/+y+mI+V9fNUhcUu2tlSrTcyWj1HygRn2n3qtu+PJF YuAdXfvLgB2Sv70T56KB2qbAB3GZHOBvf4yYnoJKZVNPu2fufnSAynlZY4ipakMm YZA5cz+PJgf/y4TH7PeDX5dLNcc0= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=mXOkQiwqNL5nH2JZWJSM5/bVNJ7o6hwmjFAIp9d62UbfaYcKaFXBADlT1/xNVn nl2+S6YsRywAagmx52t+osB2Wu48GPpX8nwL/62AZhd1YbH0Wrv0vtUv3DwQ97X9 ZuipJErtJHzpebPd56adMWPVpUHvRD6BRfts1yZnJujzQ=; Received: (qmail 4766 invoked by alias); 4 Jun 2012 17:55:22 -0000 Received: (qmail 4713 invoked by uid 22791); 4 Jun 2012 17:55:19 -0000 X-SWARE-Spam-Status: No, hits=-7.4 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, SPF_HELO_PASS, TW_CX, TW_DD, TW_OV, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 04 Jun 2012 17:55:00 +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 q54HsxLT025451 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 4 Jun 2012 13:55:00 -0400 Received: from dhcp-5-241.str.redhat.com (ovpn-116-64.ams2.redhat.com [10.36.116.64]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q54Hsvxs029649 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 4 Jun 2012 13:54:58 -0400 Message-ID: <4FCCF671.6000403@redhat.com> Date: Mon, 04 Jun 2012 19:54:57 +0200 From: Florian Weimer User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 MIME-Version: 1.0 To: Jakub Jelinek CC: libstdc++@gcc.gnu.org, GCC Patches Subject: Re: _FORTIFY_SOURCE for std::vector References: <4FC8A0DE.7030104@redhat.com> <20120601113439.GJ24904@tucnak.redhat.com> In-Reply-To: <20120601113439.GJ24904@tucnak.redhat.com> X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On 06/01/2012 01:34 PM, Jakub Jelinek wrote: > Have you looked at the assembly differences with this in? It's not great. Here's an example: void write(std::vector& blob, unsigned n, float v1, float v2, float v3, float v4) { blob[n] = v1; blob[n + 1] = v2; blob[n + 2] = v3; blob[n + 3] = v4; } Without checks, it turns into: movq (%rdi), %rax movl %esi, %edx movss %xmm0, (%rax,%rdx,4) leal 1(%rsi), %edx movss %xmm1, (%rax,%rdx,4) leal 2(%rsi), %edx movss %xmm2, (%rax,%rdx,4) leal 3(%rsi), %edx movss %xmm3, (%rax,%rdx,4) ret With checks, it's: subq $8, %rsp movq (%rdi), %rdx movq 8(%rdi), %rax movl %esi, %ecx subq %rdx, %rax sarq $2, %rax cmpq %rax, %rcx jae .L8 movss %xmm0, (%rdx,%rcx,4) leal 1(%rsi), %ecx cmpq %rax, %rcx jae .L8 movss %xmm1, (%rdx,%rcx,4) leal 2(%rsi), %ecx cmpq %rax, %rcx jae .L8 movss %xmm2, (%rdx,%rcx,4) leal 3(%rsi), %ecx cmpq %rax, %rcx jae .L8 movss %xmm3, (%rdx,%rcx,4) addq $8, %rsp ret .L8: call __chk_fail For char/signed char/unsigned char, it's even worse because there's an additional pointer load per store (bringing up the total to two—the compiler cannot see that the _M_start pointer cannot point into the std::vector object). I'm no longer sure that's acceptable overhead. We really don't want programmers to switch off _FORTIFY_SOURCE because of its overhead. I'm attaching the patch just in case. I'm not sure the test case actually ran (if it did, it passed on first try, which would be unusual.) Index: libstdc++-v3/include/bits/c++config =================================================================== --- libstdc++-v3/include/bits/c++config (revision 187951) +++ libstdc++-v3/include/bits/c++config (working copy) @@ -370,6 +370,13 @@ } while (false) #endif +// _FORTIFY_SOURCE support. Only available on GNU libc. + +namespace __gnu_cxx +{ + extern "C" void __chk_fail () __attribute__((noreturn)); +} + // Macros for race detectors. // _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(A) and // _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(A) should be used to explain Index: libstdc++-v3/include/bits/stl_vector.h =================================================================== --- libstdc++-v3/include/bits/stl_vector.h (revision 187951) +++ libstdc++-v3/include/bits/stl_vector.h (working copy) @@ -768,7 +768,10 @@ */ reference operator[](size_type __n) - { return *(this->_M_impl._M_start + __n); } + { + _M_fortify_range_check(__n); + return *(this->_M_impl._M_start + __n); + } /** * @brief Subscript access to the data contained in the %vector. @@ -783,7 +786,10 @@ */ const_reference operator[](size_type __n) const - { return *(this->_M_impl._M_start + __n); } + { + _M_fortify_range_check(__n); + return *(this->_M_impl._M_start + __n); + } protected: /// Safety check used only from at(). @@ -794,6 +800,17 @@ __throw_out_of_range(__N("vector::_M_range_check")); } + /// Range check used by operator[]. + /// No-op unless _FORTIFY_SOURCE is enabled. + void + _M_fortify_range_check(size_type __n) const + { +#if defined __USE_FORTIFY_LEVEL && __USE_FORTIFY_LEVEL > 0 + if (__n >= this->size()) + __gnu_cxx::__chk_fail(); +#endif + } + public: /** * @brief Provides access to the data contained in the %vector. Index: libstdc++-v3/testsuite/23_containers/vector/element_access/2.cc =================================================================== --- libstdc++-v3/testsuite/23_containers/vector/element_access/2.cc (revision 0) +++ libstdc++-v3/testsuite/23_containers/vector/element_access/2.cc (revision 0) @@ -0,0 +1,100 @@ +// Copyright (C) 2012 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// 23.2.4 vector + +// { dg-add-options no_pch } +// { dg-target *-*-linux* } + +#undef _FORTIFY_SOURCE +#define _FORTIFY_SOURCE 2 + +#include + +#include +#include +#include +#include + +#include + +// Testing pattern follows debug/test-strcpy_chk.c in GNU libc. +volatile int chk_fail_ok; +jmp_buf chk_fail_buf; + +static void +handler(int) +{ + if (chk_fail_ok) + { + chk_fail_ok = 0; + longjmp(chk_fail_buf, 1); + } + else + _exit(127); +} + +static void +run_test(void (*t)()) +{ + bool test __attribute__((unused)) = true; + if (setjmp(chk_fail_buf) == 0) + { + t(); + VERIFY(false); + } +} + +void test01() +{ + std::vector v(5); + v[0]; + v[4]; + chk_fail_ok = 1; + v[5]; +} + +void test02() +{ + std::vector u(5); + const std::vector& v(u); + v[0]; + v[4]; + chk_fail_ok = 1; + v[5]; +} + +int main() +{ + // See debug/test-strcpy_chk.c in GNU libc. + struct sigaction sa; + sa.sa_handler = andler; + sa.sa_flags = 0; + sigaction(SIGABRT, &sa, NULL); + int fd = open(_PATH_DEVNULL, O_WRONLY); + if (fd == -1) + close(STDERR_FILENO); + else + { + dup2(fd, STDERR_FILENO); + close(fd); + } + setenv("LIBC_FATAL_STDERR_", "1", 1); // do not use /dev/tty + run_test(test01); + run_test(test02); + return 0; +}