From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89554 invoked by alias); 23 Sep 2015 11:08:57 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 89539 invoked by uid 89); 23 Sep 2015 11:08:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 23 Sep 2015 11:08:56 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 3AF7EAB851 for ; Wed, 23 Sep 2015 11:08:55 +0000 (UTC) Received: from localhost.localdomain (vpn1-5-31.ams2.redhat.com [10.36.5.31]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8NB8rP4004743; Wed, 23 Sep 2015 07:08:54 -0400 Subject: Re: [ubsan PATCH] Fix uninitialized var issue (PR sanitizer/64906) To: Marek Polacek , GCC Patches , Jakub Jelinek References: <20150922151142.GO27588@redhat.com> From: Bernd Schmidt Message-ID: <56028845.4020708@redhat.com> Date: Wed, 23 Sep 2015 11:19:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150922151142.GO27588@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg01726.txt.bz2 On 09/22/2015 05:11 PM, Marek Polacek wrote: > diff --git gcc/c-family/c-ubsan.c gcc/c-family/c-ubsan.c > index e0cce84..d2bc264 100644 > --- gcc/c-family/c-ubsan.c > +++ gcc/c-family/c-ubsan.c > @@ -104,6 +104,7 @@ ubsan_instrument_division (location_t loc, tree op0, tree op1) > } > } > t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op0), t); > + t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op1), t); > if (flag_sanitize_undefined_trap_on_error) > tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); > else I really don't know this code, but just before the location you're patching, there's this: /* In case we have a SAVE_EXPR in a conditional context, we need to make sure it gets evaluated before the condition. If the OP0 is an instrumented array reference, mark it as having side effects so it's not folded away. */ if (flag_sanitize & SANITIZE_BOUNDS) { tree xop0 = op0; while (CONVERT_EXPR_P (xop0)) xop0 = TREE_OPERAND (xop0, 0); if (TREE_CODE (xop0) == ARRAY_REF) { TREE_SIDE_EFFECTS (xop0) = 1; TREE_SIDE_EFFECTS (op0) = 1; } } Does that need to be done for op1 as well? (I really wonder why this is needed or whether it's sufficient to find such an ARRAY_REF if you can have more complex operands). The same pattern occurs in another function, so it may be best to break it out into a new function if additional occurrences are necessary. Bernd