Re: C++/v3 PATCH to add/throw std::bad_array_new_length
On 05/06/2013 05:56 PM, Jason Merrill wrote: On 05/06/2013 08:46 AM, Florian Weimer wrote: On 05/06/2013 02:39 PM, Jason Merrill wrote: On 05/06/2013 05:46 AM, Florian Weimer wrote: Nice, this is simpler than expected. However, it makes the call sites even more bloated. Hmm, perhaps the checking should be wrapped in an inline function, so that the inliner can decide whether or not to expand it at the call site... Or we could call __cxa_vec_new[23] and rely on the check there True. The problem with using those is the indirect calls to the (possibly inline) constructors, though it might be worth doing conditionally. Would you be interested in working on that change? Yes, but it's probably better if you commit your patch right away. (in most cases—for new T[a][b], we'd still need a separate overflow check). But new T[a][b] is ill-formed, so we don't need to handle that case. I meant with one of a or b as a constant (I can't remember which it is). We still have to perform one multiplication inline, to get the total number of elements, and that needs overflow checking as well. -- Florian Weimer / Red Hat Product Security Team
Re: C++/v3 PATCH to add/throw std::bad_array_new_length
On 05/03/2013 10:24 PM, Jason Merrill wrote: Last year Florian fixed the compiler to detect overflow in array new size calculations and pass (size_t)-1 in that case. But C++11 specifies that in case of overflow the program throws std::bad_array_new_length (http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#624), so I've adjusted the checking code accordingly. Nice, this is simpler than expected. However, it makes the call sites even more bloated. This patch also adds the type to libsupc++, and several exports to libstdc++. There's also overflow checking inside __cxa_vec_new[23]. At this point, we don't know if the caller was compiled in C++11 mode. But for C++03 code, throwing a subclass of std::bad_alloc probably won't hurt. I noticed you use throw() in the declaration of std::bad_array_new_length and _GLIBCXX_USE_NOEXCEPT in the definition, which seems rather odd. I'm surprised that this even compiles. -- Florian Weimer / Red Hat Product Security Team
Re: C++/v3 PATCH to add/throw std::bad_array_new_length
On 05/06/2013 05:46 AM, Florian Weimer wrote: Nice, this is simpler than expected. However, it makes the call sites even more bloated. Hmm, perhaps the checking should be wrapped in an inline function, so that the inliner can decide whether or not to expand it at the call site... This patch also adds the type to libsupc++, and several exports to libstdc++. There's also overflow checking inside __cxa_vec_new[23]. At this point, we don't know if the caller was compiled in C++11 mode. But for C++03 code, throwing a subclass of std::bad_alloc probably won't hurt. And we never use __cxa_vec_new* anyway, they're only there because the ABI requires them. I noticed you use throw() in the declaration of std::bad_array_new_length and _GLIBCXX_USE_NOEXCEPT in the definition, which seems rather odd. I'm surprised that this even compiles. 15.4 [except.spec]/3: Two exception-specifications are compatible if: — both are non-throwing (see below), regardless of their form ... Jason
Re: C++/v3 PATCH to add/throw std::bad_array_new_length
On 05/06/2013 02:39 PM, Jason Merrill wrote: On 05/06/2013 05:46 AM, Florian Weimer wrote: Nice, this is simpler than expected. However, it makes the call sites even more bloated. Hmm, perhaps the checking should be wrapped in an inline function, so that the inliner can decide whether or not to expand it at the call site... Or we could call __cxa_vec_new[23] and rely on the check there (in most cases—for new T[a][b], we'd still need a separate overflow check). This patch also adds the type to libsupc++, and several exports to libstdc++. There's also overflow checking inside __cxa_vec_new[23]. At this point, we don't know if the caller was compiled in C++11 mode. But for C++03 code, throwing a subclass of std::bad_alloc probably won't hurt. And we never use __cxa_vec_new* anyway, they're only there because the ABI requires them. EDG-derived compilers will call it, so we should fix it as well. I noticed you use throw() in the declaration of std::bad_array_new_length and _GLIBCXX_USE_NOEXCEPT in the definition, which seems rather odd. I'm surprised that this even compiles. 15.4 [except.spec]/3: Two exception-specifications are compatible if: — both are non-throwing (see below), regardless of their form ... Thanks, I suspected as much. -- Florian Weimer / Red Hat Product Security Team
Re: C++/v3 PATCH to add/throw std::bad_array_new_length
On 05/06/2013 08:46 AM, Florian Weimer wrote: On 05/06/2013 02:39 PM, Jason Merrill wrote: On 05/06/2013 05:46 AM, Florian Weimer wrote: Nice, this is simpler than expected. However, it makes the call sites even more bloated. Hmm, perhaps the checking should be wrapped in an inline function, so that the inliner can decide whether or not to expand it at the call site... Or we could call __cxa_vec_new[23] and rely on the check there True. The problem with using those is the indirect calls to the (possibly inline) constructors, though it might be worth doing conditionally. Would you be interested in working on that change? (in most cases—for new T[a][b], we'd still need a separate overflow check). But new T[a][b] is ill-formed, so we don't need to handle that case. This patch also adds the type to libsupc++, and several exports to libstdc++. There's also overflow checking inside __cxa_vec_new[23]. At this point, we don't know if the caller was compiled in C++11 mode. But for C++03 code, throwing a subclass of std::bad_alloc probably won't hurt. And we never use __cxa_vec_new* anyway, they're only there because the ABI requires them. EDG-derived compilers will call it, so we should fix it as well. Right. Jason
C++/v3 PATCH to add/throw std::bad_array_new_length
Last year Florian fixed the compiler to detect overflow in array new
size calculations and pass (size_t)-1 in that case. But C++11 specifies
that in case of overflow the program throws std::bad_array_new_length
(http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#624), so
I've adjusted the checking code accordingly.
This patch also adds the type to libsupc++, and several exports to
libstdc++.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 9c81583687db4d3678ac2e7ee78c7c15076e7ac4
Author: Jason Merrill ja...@redhat.com
Date: Thu May 2 11:31:05 2013 -0400
Core 624/N2932: Throw bad_array_new_length on overflow
in array new size calculation.
libstdc++-v3/
* libsupc++/new: Add std::bad_array_new_length.
* libsupc++/bad_array_new.cc: New.
* libsupc++/eh_aux_runtime.cc: Add __cxa_bad_array_new_length.
* libsupc++/Makefile.in: Build them.
* config/abi/pre/gnu.ver: Add new symbols.
* config/abi/pre/gnu-versioned-namespace.ver: Add new symbols.
gcc/cp/
* init.c (throw_bad_array_new_length): New.
(build_new_1): Use it. Don't warn about braced-init-list.
(build_vec_init): Use it.
* call.c (build_operator_new_call): Use it.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 88bf100..bd8f531 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -3951,8 +3951,13 @@ build_operator_new_call (tree fnname, vectree, va_gc **args,
*fn = NULL_TREE;
/* Set to (size_t)-1 if the size check fails. */
if (size_check != NULL_TREE)
-*size = fold_build3 (COND_EXPR, sizetype, size_check,
- original_size, TYPE_MAX_VALUE (sizetype));
+{
+ tree errval = TYPE_MAX_VALUE (sizetype);
+ if (cxx_dialect = cxx11)
+ errval = throw_bad_array_new_length ();
+ *size = fold_build3 (COND_EXPR, sizetype, size_check,
+ original_size, errval);
+}
vec_safe_insert (*args, 0, *size);
*args = resolve_args (*args, complain);
if (*args == NULL)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 78fd56b..dec7390 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5357,6 +5357,7 @@ extern tree build_value_init (tree, tsubst_flags_t);
extern tree build_value_init_noctor (tree, tsubst_flags_t);
extern tree build_offset_ref (tree, tree, bool,
tsubst_flags_t);
+extern tree throw_bad_array_new_length (void);
extern tree build_new(vectree, va_gc **, tree, tree,
vectree, va_gc **, int,
tsubst_flags_t);
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 3587b08..73186eb 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -2170,6 +2170,21 @@ diagnose_uninitialized_cst_or_ref_member (tree type, bool using_new, bool compla
return diagnose_uninitialized_cst_or_ref_member_1 (type, type, using_new, complain);
}
+/* Call __cxa_bad_array_new_length to indicate that the size calculation
+ overflowed. Pretend it returns sizetype so that it plays nicely in the
+ COND_EXPR. */
+
+tree
+throw_bad_array_new_length (void)
+{
+ tree fn = get_identifier (__cxa_bad_array_new_length);
+ if (!get_global_value_if_present (fn, fn))
+fn = push_throw_library_fn (fn, build_function_type_list (sizetype,
+ NULL_TREE));
+
+ return build_cxx_call (fn, 0, NULL, tf_warning_or_error);
+}
+
/* Generate code for a new-expression, including calling the operator
new function, initializing the object, and, if an exception occurs
during construction, cleaning up. The arguments are as for
@@ -2472,9 +2487,12 @@ build_new_1 (vectree, va_gc **placement, tree type, tree nelts,
outer_nelts_check = NULL_TREE;
}
/* Perform the overflow check. */
+ tree errval = TYPE_MAX_VALUE (sizetype);
+ if (cxx_dialect = cxx11)
+ errval = throw_bad_array_new_length ();
if (outer_nelts_check != NULL_TREE)
size = fold_build3 (COND_EXPR, sizetype, outer_nelts_check,
-size, TYPE_MAX_VALUE (sizetype));
+size, errval);
/* Create the argument list. */
vec_safe_insert (*placement, 0, size);
/* Do name-lookup to find the appropriate operator. */
@@ -2699,12 +2717,8 @@ build_new_1 (vectree, va_gc **placement, tree type, tree nelts,
domain = compute_array_index_type (NULL_TREE, nelts,
complain);
else
- {
- domain = NULL_TREE;
- if (CONSTRUCTOR_NELTS (vecinit) 0)
- warning (0, non-constant array size in new, unable
- to verify length of initializer-list);
- }
+ /* We'll check the length at runtime. */
+ domain = NULL_TREE;
arraytype = build_cplus_array_type (type, domain);
vecinit = digest_init (arraytype, vecinit, complain);
}
@@ -3291,6 +3305,7 @@ build_vec_init (tree base, tree maxindex, tree init,
tree obase = base;
bool xvalue = false;
bool errors = false;
+ tree length_check = NULL_TREE;
if (TREE_CODE (atype) == ARRAY_TYPE TYPE_DOMAIN (atype))
