From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 64152 invoked by alias); 10 Oct 2016 08:59:30 -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 64131 invoked by uid 89); 10 Oct 2016 08:59:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.3 required=5.0 tests=AWL,BAYES_50,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=undesirable, fallout, GC-ed, vartracking X-HELO: mail-qk0-f172.google.com Received: from mail-qk0-f172.google.com (HELO mail-qk0-f172.google.com) (209.85.220.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 10 Oct 2016 08:59:19 +0000 Received: by mail-qk0-f172.google.com with SMTP id o68so98165913qkf.3 for ; Mon, 10 Oct 2016 01:59:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=+l1SDbSekgGpg0w9fwhfj8/3AS4Doo5tjWKKGZ320F0=; b=iNtZjCAbg4nWmijkwP1apiwQS1e+Sgoe1VXVSzG1dKwmKtb2w8I9BlScT9Ag1/Lr3U nDFEJIpq9sU+Qd9RQunM+WLR1dPIGDy9ebSWk+r6YnAbYY95GOcJQ3LKDzZtI3BHqOzZ 0E0hVw/ijQbr0AALoDNPFf8jcsftdamhToAbRY9Cv8SqTMlnEmim5iPQF6XSAgFlOJoB zjeDoaF2R8z28ovjmEnalFaycSucEe0RJQcdar8y3TvahcnaAPAXprUdc1LXNkMnXqYb CJD/gmlaGUHwxGqGW34ok6/Wakbha26cQtVAyq2cAKy10kmCiynq8NkK5KsZ2WoC0qaA BI5A== X-Gm-Message-State: AA6/9RnE7H+CFqXDfoVglBjghLLo9xf1kbN/AGOhBTF/Lf4/aLsOcrdPvyWkPg2TXKSyF2MofE61QMcx5Mo0nQ== X-Received: by 10.194.62.205 with SMTP id a13mr2820393wjs.89.1476089957354; Mon, 10 Oct 2016 01:59:17 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.155.146 with HTTP; Mon, 10 Oct 2016 01:59:16 -0700 (PDT) In-Reply-To: <1863165.r8qPLI7fxq@polaris> References: <1863165.r8qPLI7fxq@polaris> From: Richard Biener Date: Mon, 10 Oct 2016 08:59:00 -0000 Message-ID: Subject: Re: [patch] Fix GC issue triggered by arithmetic overflow checking To: Eric Botcazou Cc: GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg00578.txt.bz2 On Sat, Oct 8, 2016 at 8:56 PM, Eric Botcazou wrote: > Hi, > > adding patterns for unsigned arithmetic overflow checking in a back-end can > have unexpected fallout because of a latent GC issue: when they are present, > GIMPLE optimization passes can create complex (math. sense) types at will by > invoking build_complex_type. Now build_complex_type goes through the type > caonicalization hashtable, which is GC-ed, so its behavior depends on the > actual collection points. > > The other type-building functions present in tree.c do the same so no big deal > but build_complex_type is special because it also does: > > /* We need to create a name, since complex is a fundamental type. */ > if (! TYPE_NAME (t)) > { > const char *name; > if (component_type == char_type_node) > name = "complex char"; > else if (component_type == signed_char_type_node) > name = "complex signed char"; > else if (component_type == unsigned_char_type_node) > name = "complex unsigned char"; > else if (component_type == short_integer_type_node) > name = "complex short int"; > else if (component_type == short_unsigned_type_node) > name = "complex short unsigned int"; > else if (component_type == integer_type_node) > name = "complex int"; > else if (component_type == unsigned_type_node) > name = "complex unsigned int"; > else if (component_type == long_integer_type_node) > name = "complex long int"; > else if (component_type == long_unsigned_type_node) > name = "complex long unsigned int"; > else if (component_type == long_long_integer_type_node) > name = "complex long long int"; > else if (component_type == long_long_unsigned_type_node) > name = "complex long long unsigned int"; > else > name = 0; > > if (name != 0) > TYPE_NAME (t) = build_decl (UNKNOWN_LOCATION, TYPE_DECL, > get_identifier (name), t); > } > > so it creates a DECL node every time a new canonical complex type is created, > bumping the DECL_UID counter in the process. Which means that the DECL_UID > counter is sensitive to the collection points, which in turn means that the > result of algorithms depending on the DECL_UID counter also is. > > This for example resulted in a bootstrap comparison failure on a SPARC/Solaris > machine doing a strict stage2/stage3 comparison because the contents of the > .debug_loc section were different: location lists computed by var-tracking > were slightly different because of a different hashing. > > I'm not sure whether the hashing done by var-tracking should be sensitive to > the DECL_UID of nodes or not, but I think that having the DECL_UID counter > depend on the collection points is highly undesirable, so the attached patch > attempts to prevent it; it at least fixed the bootstrap comparison failure. I believe the rule is that you might only depend on the order of objects with respect to their DECL_UID, not the actual value of the DECL_UID. As var-tracking shouldn't look at TYPE_DECLs (?) it's probably a latent var-tracking bug as well. > Tested on x86_64-suse-linux, OK for the mainline? I'd prefer the named parameter to be defaulted to false and the few places in the FEs fixed (eventually that name business should be handled like names for nodes like integer_type_node -- I see no reason why build_complex_type should have this special-case at all! That is, why are the named vairants in the type hash in the first place?) Richard. > > 2016-10-08 Eric Botcazou > > * tree.h (build_complex_type): Add second parameter with default. > * builtins.c (expand_builtin_cexpi): Pass false in call to above. > (fold_builtin_sincos): Likewise. > (fold_builtin_arith_overflow): Likewise. > * gimple-fold.c (fold_builtin_atomic_compare_exchange): Likewise. > (gimple_fold_call): Likewise. > * stor-layout.c (bitwise_type_for_mode): Likewise. > * tree-ssa-dce.c (maybe_optimize_arith_overflow): Likewise. > * tree-ssa-math-opts.c (match_uaddsub_overflow): Likewise. > * tree.c (build_complex): Likewise. > (build_complex_type): Add NAMED second parameter and adjust recursive > call. Create a TYPE_DECL only if NAMED is true. > > -- > Eric Botcazou