On Wed, Jun 12, 2013 at 05:29:21PM +0200, Jakub Jelinek wrote:
> > @@ -4070,8 +4081,12 @@ cp_build_binary_op (location_t location,
> > {
> > enum tree_code tcode0 = code0, tcode1 = code1;
> > tree cop1 = fold_non_dependent_expr_sfinae (op1, tf_none);
> > + cop1 = maybe_constant_
On Wed, Jun 12, 2013 at 05:17:45PM +0200, Marek Polacek wrote:
> @@ -3867,6 +3868,7 @@ cp_build_binary_op (location_t location,
>tree final_type = 0;
>
>tree result;
> + tree orig_type = NULL;
>
>/* Nonzero if this is an operation like MIN or MAX which can
> safely be compute
On Wed, Jun 12, 2013 at 03:52:08PM +0200, Jakub Jelinek wrote:
> No, you really need to use the cp_save_expr/c_save_expr, especially for
> C it e.g. fully folds etc. You want to call that in
> cp_build_binary_op etc., also because you want both the instrument_expr
> itself, but also the original b
On Wed, Jun 12, 2013 at 03:48:24PM +0200, Marek Polacek wrote:
> On Tue, Jun 11, 2013 at 10:44:12PM +0200, Jakub Jelinek wrote:
> > There is another thing to solve BTW, op0 and/or op1 might have side-effects,
> > if you are going to evaluate them more than once, they need to be surrounded
> > into
On Tue, Jun 11, 2013 at 10:44:12PM +0200, Jakub Jelinek wrote:
> There is another thing to solve BTW, op0 and/or op1 might have side-effects,
> if you are going to evaluate them more than once, they need to be surrounded
> into cp_save_expr resp. c_save_expr.
There's that unpleasant thing that cp_
On Tue, Jun 11, 2013 at 10:44:12PM +0200, Jakub Jelinek wrote:
> There is another thing to solve BTW, op0 and/or op1 might have side-effects,
> if you are going to evaluate them more than once, they need to be surrounded
> into cp_save_expr resp. c_save_expr.
I see. Thanks for the notice.
On Tue, Jun 11, 2013 at 10:40:12PM +0200, Marek Polacek wrote:
> On Tue, Jun 11, 2013 at 10:33:25PM +0200, Jakub Jelinek wrote:
> > That means you probably should move the function call down in
> > cp_build_binary_op (resp. C counterpart), after the arguments are converted
> > to result_type?
>
>
On Tue, Jun 11, 2013 at 10:33:25PM +0200, Jakub Jelinek wrote:
> That means you probably should move the function call down in
> cp_build_binary_op (resp. C counterpart), after the arguments are converted
> to result_type?
Ok, certainly. Seems the arguments are converted here:
if (! converted)
On Tue, Jun 11, 2013 at 10:20:24PM +0200, Marek Polacek wrote:
> On Tue, Jun 11, 2013 at 10:09:00PM +0200, Jakub Jelinek wrote:
> > On Tue, Jun 11, 2013 at 09:44:40PM +0200, Marek Polacek wrote:
> > > > >+ tree type0 = TREE_TYPE (op0);
> > > > >+ tree type1 = TREE_TYPE (op1);
> > > >
> > > > Can
On Tue, Jun 11, 2013 at 10:09:00PM +0200, Jakub Jelinek wrote:
> On Tue, Jun 11, 2013 at 09:44:40PM +0200, Marek Polacek wrote:
> > > >+ tree type0 = TREE_TYPE (op0);
> > > >+ tree type1 = TREE_TYPE (op1);
> > >
> > > Can the 2 types be different? I thought divisions had homogeneous
> > > argume
On Tue, Jun 11, 2013 at 09:44:40PM +0200, Marek Polacek wrote:
> > >+ tree type0 = TREE_TYPE (op0);
> > >+ tree type1 = TREE_TYPE (op1);
> >
> > Can the 2 types be different? I thought divisions had homogeneous
> > arguments, and the instrumentation was done late enough to avoid any
> > potentia
Hi!
On Tue, Jun 11, 2013 at 09:14:36PM +0200, Marc Glisse wrote:
> Hello,
>
> couple comments (not a true review)
Thanks anyway ;).
> On Tue, 11 Jun 2013, Marek Polacek wrote:
>
> >+tree
> >+ubsan_instrument_division (location_t loc, tree op0, tree op1)
> >+{
> >+ tree t, tt;
> >+ tree type0
Hello,
couple comments (not a true review)
On Tue, 11 Jun 2013, Marek Polacek wrote:
+tree
+ubsan_instrument_division (location_t loc, tree op0, tree op1)
+{
+ tree t, tt;
+ tree type0 = TREE_TYPE (op0);
+ tree type1 = TREE_TYPE (op1);
Can the 2 types be different? I thought divisions had
On Sat, Jun 08, 2013 at 08:22:33PM +0200, Jakub Jelinek wrote:
> > >+ tt = fold_build2 (EQ_EXPR, boolean_type_node, op1,
> > >+ integer_minus_one_node);
> >
> > Don't we usually try to have both operands of a comparison of the
> > same type?
>
> Not just usually, it really has to be
On Mon, Jun 10, 2013 at 02:29:25PM +, Joseph S. Myers wrote:
> On Sat, 8 Jun 2013, Marek Polacek wrote:
>
> > + if (code == LSHIFT_EXPR
> > + && !TYPE_UNSIGNED (TREE_TYPE (op0))
> > + && (flag_isoc99 || flag_isoc11))
>
> flag_isoc11 implies flag_isoc99, you only need to check flag_
On Sat, 8 Jun 2013, Marek Polacek wrote:
> + if (code == LSHIFT_EXPR
> + && !TYPE_UNSIGNED (TREE_TYPE (op0))
> + && (flag_isoc99 || flag_isoc11))
flag_isoc11 implies flag_isoc99, you only need to check flag_isoc99 here.
--
Joseph S. Myers
jos...@codesourcery.com
On Mon, Jun 10, 2013 at 11:32:22AM +0200, Jakub Jelinek wrote:
> On Mon, Jun 10, 2013 at 11:24:16AM +0200, Marek Polacek wrote:
> > > >@@ -4070,8 +4077,15 @@ cp_build_binary_op (location_t location,
> > > > {
> > > > enum tree_code tcode0 = code0, tcode1 = code1;
> > > > tree cop1 = fold_non_de
On Mon, Jun 10, 2013 at 11:24:16AM +0200, Marek Polacek wrote:
> > >@@ -4070,8 +4077,15 @@ cp_build_binary_op (location_t location,
> > > {
> > > enum tree_code tcode0 = code0, tcode1 = code1;
> > > tree cop1 = fold_non_dependent_expr_sfinae (op1, tf_none);
> > >+cop1 = maybe_constant
On Sat, Jun 08, 2013 at 07:48:27PM +0200, Marc Glisse wrote:
> >+ tt = fold_build2 (EQ_EXPR, boolean_type_node, op1,
> >+integer_minus_one_node);
>
> Don't we usually try to have both operands of a comparison of the
> same type?
Will fix.
> >+ t = fold_build2 (EQ_EXPR, boolean_
On Sat, Jun 08, 2013 at 07:48:27PM +0200, Marc Glisse wrote:
> >+/* Instrument division by zero and INT_MIN / -1. */
> >+
> >+tree
> >+ubsan_instrument_division (location_t loc, enum tree_code code,
> >+ tree op0, tree op1)
> >+{
> >+ tree t, tt;
> >+ tree orig = build2 (co
Hello,
thanks for working on this. Just a few questions inline:
On Sat, 8 Jun 2013, Marek Polacek wrote:
+/* Instrument division by zero and INT_MIN / -1. */
+
+tree
+ubsan_instrument_division (location_t loc, enum tree_code code,
+ tree op0, tree op1)
+{
+ tree t, t
Thanks for the reviews, here is another version. I haven't touched
the division by zero instrumentation, but the shift instrumentation is
revamped; what it should instrument now is, as Jakub wrote:
1) if ((unsigned) y > precm1) ub
plus for signed x << y:
2) for C99/C11 if ((unsigned) x >> (precm1
On Wed, Jun 05, 2013 at 07:50:52PM +, Joseph S. Myers wrote:
> On Wed, 5 Jun 2013, Marek Polacek wrote:
>
> > It works by creating a COMPOUND_EXPR around original expression, so e.g.
> > it creates:
> >
> > if (b < 0 || (b > 31 || a < 0))
> > {
> > __builtin___ubsan_handle_shift_out_of_
On Thu, Jun 06, 2013 at 03:26:19PM +0200, Segher Boessenkool wrote:
> >The C++11/C++14 undefined behavior of left signed shift can be tested
> >similarly, if ((unsigned type for op0's type) op0) >> (precm1 - y)
> >is greater than one, then it is undefined behavior.
> >Jason, does
> >http://www.open
The C++11/C++14 undefined behavior of left signed shift can be tested
similarly, if ((unsigned type for op0's type) op0) >> (precm1 - y)
is greater than one, then it is undefined behavior.
Jason, does
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/
n3675.html#1457
apply just to C++11/C+
On 06/06/2013 02:07 AM, Jakub Jelinek wrote:
Jason, does
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3675.html#1457
apply just to C++11/C++14, or to C++03 too?
The committee hasn't said anything about which DRs since C++03 apply to
it. I take the position that most do, but not th
[Resending with less text/html]
On Thu, Jun 6, 2013 at 1:55 AM, Konstantin Serebryany
wrote:
> On Thu, Jun 6, 2013 at 12:44 PM, Jakub Jelinek wrote:
> > On Thu, Jun 06, 2013 at 12:41:56PM +0400, Konstantin Serebryany wrote:
> >> As for libstdc++, I completely agree, we don't want to depend on it
On Thu, Jun 6, 2013 at 12:59 PM, Jakub Jelinek wrote:
> On Thu, Jun 06, 2013 at 12:55:17PM +0400, Konstantin Serebryany wrote:
>> > ubsan actually needs
>> > U _ZTIN10__cxxabiv117__class_type_infoE@@CXXABI_1.3
>> > U _ZTIN10__cxxabiv120__si_class_type_infoE@@CXXAB
On Thu, Jun 06, 2013 at 12:55:17PM +0400, Konstantin Serebryany wrote:
> > ubsan actually needs
> > U _ZTIN10__cxxabiv117__class_type_infoE@@CXXABI_1.3
> > U _ZTIN10__cxxabiv120__si_class_type_infoE@@CXXABI_1.3
> > U _ZTIN10__cxxabiv121__vmi_class_
On Thu, Jun 6, 2013 at 12:44 PM, Jakub Jelinek wrote:
> On Thu, Jun 06, 2013 at 12:41:56PM +0400, Konstantin Serebryany wrote:
>> As for libstdc++, I completely agree, we don't want to depend on it,
>> and we don't.
>
> ubsan actually needs
> U _ZTIN10__cxxabiv117__class_type_info
+rich...@metafoo.co.uk
On Thu, Jun 6, 2013 at 12:21 PM, Jakub Jelinek wrote:
> On Thu, Jun 06, 2013 at 11:46:19AM +0400, Konstantin Serebryany wrote:
>> If we are going to import the ubsan run-time from LLVM's
>> projects/compiler-rt/lib/ubsan,
>> we may also need to update the contents of
>> lib
On Thu, Jun 06, 2013 at 12:41:56PM +0400, Konstantin Serebryany wrote:
> As for libstdc++, I completely agree, we don't want to depend on it,
> and we don't.
ubsan actually needs
U _ZTIN10__cxxabiv117__class_type_infoE@@CXXABI_1.3
U _ZTIN10__cxxabiv120__si_class_t
On Thu, Jun 6, 2013 at 12:26 PM, Andrew Pinski wrote:
> On Thu, Jun 6, 2013 at 1:21 AM, Jakub Jelinek wrote:
>> On Thu, Jun 06, 2013 at 11:46:19AM +0400, Konstantin Serebryany wrote:
>>> If we are going to import the ubsan run-time from LLVM's
>>> projects/compiler-rt/lib/ubsan,
>>> we may also n
On Thu, Jun 06, 2013 at 01:26:06AM -0700, Andrew Pinski wrote:
> On Thu, Jun 6, 2013 at 1:21 AM, Jakub Jelinek wrote:
> > On Thu, Jun 06, 2013 at 11:46:19AM +0400, Konstantin Serebryany wrote:
> >> If we are going to import the ubsan run-time from LLVM's
> >> projects/compiler-rt/lib/ubsan,
> >> w
On Thu, Jun 6, 2013 at 1:21 AM, Jakub Jelinek wrote:
> On Thu, Jun 06, 2013 at 11:46:19AM +0400, Konstantin Serebryany wrote:
>> If we are going to import the ubsan run-time from LLVM's
>> projects/compiler-rt/lib/ubsan,
>> we may also need to update the contents of
>> libsanitizer/sanitizer_commo
On Wed, Jun 5, 2013 at 11:40 PM, Andrew Pinski wrote:
> On Wed, Jun 5, 2013 at 12:23 PM, Jakub Jelinek wrote:
>> On Wed, Jun 05, 2013 at 11:44:07AM -0700, Andrew Pinski wrote:
>>> On Wed, Jun 5, 2013 at 10:57 AM, Marek Polacek wrote:
>>> > Comments, please?
>>> I think it might be better to do h
On Wed, Jun 05, 2013 at 09:35:08PM +0200, Jakub Jelinek wrote:
> On Wed, Jun 05, 2013 at 09:19:10PM +0200, Jakub Jelinek wrote:
> > On Wed, Jun 05, 2013 at 07:57:28PM +0200, Marek Polacek wrote:
> > > + tree t, tt;
> > > + tree orig = build2 (code, TREE_TYPE (op0), op0, op1);
> > > + tree prec =
On Wed, 5 Jun 2013, Jakub Jelinek wrote:
> On Wed, Jun 05, 2013 at 11:44:07AM -0700, Andrew Pinski wrote:
> > On Wed, Jun 5, 2013 at 10:57 AM, Marek Polacek wrote:
> > > Comments, please?
> > I think it might be better to do handle this while gimplification
> > happens rather than while parsing.
On Wed, 5 Jun 2013, Marek Polacek wrote:
> It works by creating a COMPOUND_EXPR around original expression, so e.g.
> it creates:
>
> if (b < 0 || (b > 31 || a < 0))
> {
> __builtin___ubsan_handle_shift_out_of_bounds ();
> }
> else
> {
> 0
> }, a << b;
>
> from original "a <<= b;
On Wed, Jun 5, 2013 at 12:23 PM, Jakub Jelinek wrote:
> On Wed, Jun 05, 2013 at 11:44:07AM -0700, Andrew Pinski wrote:
>> On Wed, Jun 5, 2013 at 10:57 AM, Marek Polacek wrote:
>> > Comments, please?
>> I think it might be better to do handle this while gimplification
>> happens rather than while
On Wed, Jun 05, 2013 at 09:19:10PM +0200, Jakub Jelinek wrote:
> On Wed, Jun 05, 2013 at 07:57:28PM +0200, Marek Polacek wrote:
> > + tree t, tt;
> > + tree orig = build2 (code, TREE_TYPE (op0), op0, op1);
> > + tree prec = build_int_cst (TREE_TYPE (op0),
> > +TYPE_PRECIS
On Wed, Jun 05, 2013 at 11:44:07AM -0700, Andrew Pinski wrote:
> On Wed, Jun 5, 2013 at 10:57 AM, Marek Polacek wrote:
> > Comments, please?
> I think it might be better to do handle this while gimplification
> happens rather than while parsing. The main reason is that constexpr
> might fail due
On Wed, Jun 05, 2013 at 07:57:28PM +0200, Marek Polacek wrote:
> There is of course a lot of stuff that needs to be done, more
> specifically:
> 0) fix an ICE which I've noticed right now ;(
> long a = 1;
> int b = 3;
> a <<= b;
> (error: mismatching comparison opera
On Wed, Jun 5, 2013 at 10:57 AM, Marek Polacek wrote:
> Hi!
>
> This is an attempt to add the Undefined Behavior Sanitizer to GCC.
> Note that it's very alpha version; so far it doesn't do that much,
> at the moment it should handle division by zero cases, INT_MIN / -1,
> and various shift cases (
Hi!
This is an attempt to add the Undefined Behavior Sanitizer to GCC.
Note that it's very alpha version; so far it doesn't do that much,
at the moment it should handle division by zero cases, INT_MIN / -1,
and various shift cases (shifting by a negative value, shifting when
second operand is >= t
45 matches
Mail list logo
