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<float>& 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.)
--
Florian Weimer / Red Hat Product Security Team
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
+// <http://www.gnu.org/licenses/>.
+
+// 23.2.4 vector
+
+// { dg-add-options no_pch }
+// { dg-target *-*-linux* }
+
+#undef _FORTIFY_SOURCE
+#define _FORTIFY_SOURCE 2
+
+#include <vector>
+
+#include <fcntl.h>
+#include <paths.h>
+#include <setjmp.h>
+#include <signal.h>
+
+#include <testsuite_hooks.h>
+
+// 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<int> v(5);
+ v[0];
+ v[4];
+ chk_fail_ok = 1;
+ v[5];
+}
+
+void test02()
+{
+ std::vector<int> u(5);
+ const std::vector<int>& 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;
+}
