| Message ID | 503364F2.6020908@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 08/21/2012 12:37 PM, Florian Weimer wrote: > I don't think there are any callers out there, but let's fix this for > completeness. > > A compiler emitting code to call this function would still have to > perform overflow checks for the new T[n][m] case, so this interface is > not as helpful as it looks at first glance. > > Tested on x86_64-redhat-linux-gnu. Ping? This function is apparently used by compilers based on the EDG front-end, so it's not actually dead.
Hi, On 09/17/2012 11:51 AM, Florian Weimer wrote: > On 08/21/2012 12:37 PM, Florian Weimer wrote: >> I don't think there are any callers out there, but let's fix this for >> completeness. >> >> A compiler emitting code to call this function would still have to >> perform overflow checks for the new T[n][m] case, so this interface is >> not as helpful as it looks at first glance. >> >> Tested on x86_64-redhat-linux-gnu. > > Ping? > > This function is apparently used by compilers based on the EDG > front-end, so it's not actually dead. Being code that touches the library, the patch should go to the libstdc++ maliling list too. Likewise the testcase, should be in the libstdc++ testsuite, I guess. That said, I didn't really follow the details of your recent work. Who did? Jason? I would gently ping the same maintainer. Thanks! Paolo.
On 09/17/2012 12:15 PM, Paolo Carlini wrote: > Hi, > > On 09/17/2012 11:51 AM, Florian Weimer wrote: >> On 08/21/2012 12:37 PM, Florian Weimer wrote: >>> I don't think there are any callers out there, but let's fix this for >>> completeness. >>> >>> A compiler emitting code to call this function would still have to >>> perform overflow checks for the new T[n][m] case, so this interface is >>> not as helpful as it looks at first glance. >>> >>> Tested on x86_64-redhat-linux-gnu. >> >> Ping? >> >> This function is apparently used by compilers based on the EDG >> front-end, so it's not actually dead. > Being code that touches the library, the patch should go to the > libstdc++ maliling list too. Likewise the testcase, should be in the > libstdc++ testsuite, I guess. Oh, I thought that this wouldn't apply to internal C++ support code. Sorry. > That said, I didn't really follow the details of your recent work. Who > did? Jason? I would gently ping the same maintainer. Indeed, Jason reviewed that. Cc:ing.
On 09/17/2012 12:54 PM, Florian Weimer wrote: > On 09/17/2012 12:15 PM, Paolo Carlini wrote: >> Hi, >> >> On 09/17/2012 11:51 AM, Florian Weimer wrote: >>> On 08/21/2012 12:37 PM, Florian Weimer wrote: >>>> I don't think there are any callers out there, but let's fix this for >>>> completeness. >>>> >>>> A compiler emitting code to call this function would still have to >>>> perform overflow checks for the new T[n][m] case, so this interface is >>>> not as helpful as it looks at first glance. >>>> >>>> Tested on x86_64-redhat-linux-gnu. >>> >>> Ping? >>> >>> This function is apparently used by compilers based on the EDG >>> front-end, so it's not actually dead. > >> Being code that touches the library, the patch should go to the >> libstdc++ maliling list too. Likewise the testcase, should be in the >> libstdc++ testsuite, I guess. > > Oh, I thought that this wouldn't apply to internal C++ support code. > Sorry. > >> That said, I didn't really follow the details of your recent work. Who >> did? Jason? I would gently ping the same maintainer. > > Indeed, Jason reviewed that. Cc:ing. Ping? Patch is at: http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01416.html Thanks, Florian
Hi, Florian Weimer <fweimer@redhat.com> ha scritto: >Ping? > >Patch is at: http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01416.html Sorry, I don't know the code well enough to review your patch, but since I'm in CC, I still don't understand why, instead of adding a full libstdc++ testcase you are extending a C++ testcase, in old-deja even, normally considered legacy. Paolo
On 10/30/2012 05:17 PM, Paolo Carlini wrote:
> Sorry, I don't know the code well enough to review your patch, but since I'm in CC, I still don't understand why, instead of adding a full libstdc++ testcase you are extending a C++ testcase, in old-deja even, normally considered legacy.
AFAIK, this is the only place we have such a test. I suppose I could it
put it into testsuite/18_support, but I would have to duplicate a bit of
the machinery of the original test case because I can't just write a
class and take the address of its constructor and destructor (whose
addresses are passed to the tested functions). I really didn't want to
do that because there are some platform dependencies (the __ARM_EABI__
#ifdef).
Not sure if this makes sense, but those were my reasons.
2012-08-21 Florian Weimer <fweimer@redhat.com> * libsupc++/vec.cc (compute_size): New function. (__cxa_vec_new2, __cxa_vec_new3): Use it. 2012-08-21 Florian Weimer <fweimer@redhat.com> * g++.old-deja/g++.abi/cxa_vec.C (test5, test6): New. diff --git a/gcc/testsuite/g++.old-deja/g++.abi/cxa_vec.C b/gcc/testsuite/g++.old-deja/g++.abi/cxa_vec.C index f3d602f..e2b82e7 100644 --- a/gcc/testsuite/g++.old-deja/g++.abi/cxa_vec.C +++ b/gcc/testsuite/g++.old-deja/g++.abi/cxa_vec.C @@ -8,7 +8,7 @@ // Avoid use of none-overridable new/delete operators in shared // { dg-options "-static" { target *-*-mingw* } } // Test __cxa_vec routines -// Copyright (C) 2000, 2005 Free Software Foundation, Inc. +// Copyright (C) 2000-2012 Free Software Foundation, Inc. // Contributed by Nathan Sidwell 7 Apr 2000 <nathan@nathan@codesourcery.com> #if defined (__GXX_ABI_VERSION) && __GXX_ABI_VERSION >= 100 @@ -255,6 +255,80 @@ void test4 () return; } +static const std::size_t large_size = std::size_t(1) << (sizeof(std::size_t) * 8 - 2); + +// allocate an array whose size causes an overflow during multiplication +void test5 () +{ + static bool started = false; + + if (!started) + { + started = true; + std::set_terminate (test0); + + ctor_count = dtor_count = 1; + dtor_repeat = false; + blocks = 0; + + try + { + void *ary = abi::__cxa_vec_new (large_size, 4, padding, ctor, dtor); + longjmp (jump, 1); + } + catch (std::bad_alloc) + { + if (ctor_count != 1) + longjmp (jump, 4); + } + catch (...) + { + longjmp (jump, 2); + } + } + else + { + longjmp (jump, 3); + } + return; +} + +// allocate an array whose size causes an overflow during addition +void test6 () +{ + static bool started = false; + + if (!started) + { + started = true; + std::set_terminate (test0); + + ctor_count = dtor_count = 1; + dtor_repeat = false; + blocks = 0; + + try + { + void *ary = abi::__cxa_vec_new (std::size_t(-1) / 4, 4, padding, ctor, dtor); + longjmp (jump, 1); + } + catch (std::bad_alloc) + { + if (ctor_count != 1) + longjmp (jump, 4); + } + catch (...) + { + longjmp (jump, 2); + } + } + else + { + longjmp (jump, 3); + } + return; +} + static void (*tests[])() = { test0, @@ -262,6 +336,8 @@ static void (*tests[])() = test2, test3, test4, + test5, + test6, NULL }; diff --git a/libstdc++-v3/libsupc++/vec.cc b/libstdc++-v3/libsupc++/vec.cc index 700c5ef..bfce117 100644 --- a/libstdc++-v3/libsupc++/vec.cc +++ b/libstdc++-v3/libsupc++/vec.cc @@ -1,6 +1,6 @@ // New abi Support -*- C++ -*- -// Copyright (C) 2000, 2001, 2003, 2004, 2009, 2011 +// Copyright (C) 2000-2012 // Free Software Foundation, Inc. // // This file is part of GCC. @@ -59,6 +59,19 @@ namespace __cxxabiv1 globals->caughtExceptions = p->nextException; globals->uncaughtExceptions += 1; } + + // Compute the total size with overflow checking. + std::size_t compute_size(std::size_t element_count, + std::size_t element_size, + std::size_t padding_size) + { + if (element_size && element_count > std::size_t(-1) / element_size) + throw std::bad_alloc(); + std::size_t size = element_count * element_size; + if (size + padding_size < size) + throw std::bad_alloc(); + return size + padding_size; + } } // Allocate and construct array. @@ -83,7 +96,8 @@ namespace __cxxabiv1 void *(*alloc) (std::size_t), void (*dealloc) (void *)) { - std::size_t size = element_count * element_size + padding_size; + std::size_t size + = compute_size(element_count, element_size, padding_size); char *base = static_cast <char *> (alloc (size)); if (!base) return base; @@ -124,7 +138,8 @@ namespace __cxxabiv1 void *(*alloc) (std::size_t), void (*dealloc) (void *, std::size_t)) { - std::size_t size = element_count * element_size + padding_size; + std::size_t size + = compute_size(element_count, element_size, padding_size); char *base = static_cast<char *>(alloc (size)); if (!base) return base;