From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-x1132.google.com (mail-yw1-x1132.google.com [IPv6:2607:f8b0:4864:20::1132]) by sourceware.org (Postfix) with ESMTPS id A0861385842E for ; Mon, 1 Aug 2022 19:15:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A0861385842E Received: by mail-yw1-x1132.google.com with SMTP id 00721157ae682-31f443e276fso119485137b3.1 for ; Mon, 01 Aug 2022 12:15:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=NP8BEH5iOF8HXWH/UGY+xnG7EWx3ukGQmjZbvMXHFQk=; b=yV4+QTfhlviL3fAhAInMb58Kf/nE3+too2szE8aV3oQ0K31GShVy8JVuOBFVwsgtg2 Nexc9mCe8Dpnw0j/rHqCw+BIZgI2d5LHNnJjIDDO3MU1NgL9KItq4X+oG81CG6qMbBvn EXXdLmPloJvDCSy68t4z6C9E9vky/jJnEI0XsnFeeQJxb9FFqsog15ox1x9SWK/FL+C8 706Jb+ebF9rOsBn3rNJliXorruy+xC+duzMy4szSJN5Vhs08uldC3Sj4pe49dGkNallz mHS5l+am/fjIQ7aWlb/kCkJp1+GLBfH+k3u11vqv5dHSC9HFj9N/IFSRlYtSUMXvHUnl HYwA== X-Gm-Message-State: ACgBeo10bgXjNOfLPTGLE35d43kzcQf8efKhBPrA/UMLCQ6dqhpUoAQI bAPtKuJdudxq8s8g3ksnG0Yc4kV6TkstGkgK9dtluQ== X-Google-Smtp-Source: AA6agR5mPHfcYfkJLtjE7sN+HzrpIE7/5ak3NTcl9vdXWjUUQWoWR3Hn1CFZhJFL5LhjZq7K2RxpQITQUsA14fA+OpA= X-Received: by 2002:a0d:df0e:0:b0:31f:661b:c201 with SMTP id i14-20020a0ddf0e000000b0031f661bc201mr14249378ywe.195.1659381346900; Mon, 01 Aug 2022 12:15:46 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Fangrui Song Date: Mon, 1 Aug 2022 12:15:35 -0700 Message-ID: Subject: Re: [PATCH] [PR83782] i386 PIE: avoid @GOTOFF for ifuncs and their aliases To: "H.J. Lu" Cc: Alexandre Oliva , GCC Patches , Jan Hubicka Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-18.6 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Aug 2022 19:15:49 -0000 On Mon, Aug 1, 2022 at 12:05 PM H.J. Lu via Gcc-patches wrote: > > On Thu, Jul 28, 2022 at 9:31 AM H.J. Lu wrote: > > > > On Thu, Jul 28, 2022 at 1:26 AM Alexandre Oliva wrote: > > > > > > On Jul 27, 2022, "H.J. Lu" wrote: > > > > > > > On Tue, Jul 26, 2022 at 10:14 PM Alexandre Oliva wrote: > > > > > > >> The use of @GOTOFF for locally-bound but externally-visible symbols > > > >> (e.g. protected visibility) also breaks pointer identity if the > > > >> canonical address ends up preempted by a PLT entry. > > > > > > > Here is a different fix: > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598667.html > > > > > > Oh, thanks, I'd missed that. > > > > > > It doesn't seem to fix the part of the problem I quoted above, though. > > > I think fixing that requires testing the visibility, to make sure the > > > symbol's canonical address cannot be preempted, which may occur with > > > local binding, if the symbol is protected and referenced in the main > > > program, otherwise pointer identity is broken again, admittedly for a > > > more obscure case, but pointer identity was the point of the PR. > > > > The protected symbol issue isn't IFUNC specific. The protected > > symbol handling is changed in glibc 2.36 and binutils 2.39. Only > > the address of the protected symbol definition should be used as > > its address. > > > > > >> * config/i386/i386.cc (ix86_call_use_plt_p): Follow the alias > > > >> chain looking for an ifunc, as in gcc.target/i386/mvc10.c. > > > > > > You may also need to do something like this bit for mvc10.c on ia32 PIE. > > > Because the ifunc is called through an alias, AFAICT we don't even > > > notice that the call target is (an alias to) an ifunc. GCC's > > > gotoff_operand predicate accepts it, but binutils (the linker, IIRC) > > > then rejects that reference, because the named symbol is an alias to an > > > ifunc. > > > > Yes, this change is needed. > > I think this fix should be applied to default_binds_local_p_3: > > if (lookup_attribute ("weakref", DECL_ATTRIBUTES (exp)) > || (!targetm.ifunc_ref_local_ok () > && TREE_CODE (exp) == FUNCTION_DECL > && cgraph_node::get (exp) > && cgraph_node::get (exp)->ifunc_resolver)) > return false; > > since the ifunc_resolver check won't work on aliases. > > -- > H.J. A note if people are going to refactor default_binds_local_p_3 or add a new default_binds_local_p_* variant (unrelated to the ifunc problem): extern_protected_data == true is now legacy. For new variants we want all extern_protected_data == false.