From patchwork Thu Jan 12 21:28:49 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rahul Chaudhry via binutils X-Patchwork-Id: 91234 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp1839634qgi; Thu, 12 Jan 2017 13:29:16 -0800 (PST) X-Received: by 10.99.22.65 with SMTP id 1mr19918918pgw.70.1484256556310; Thu, 12 Jan 2017 13:29:16 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id n5si10398112pgh.185.2017.01.12.13.29.16 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Jan 2017 13:29:16 -0800 (PST) Received-SPF: pass (google.com: domain of binutils-return-95389-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org; spf=pass (google.com: domain of binutils-return-95389-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=binutils-return-95389-patch=linaro.org@sourceware.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:from:reply-to:date:message-id :subject:to:cc:content-type; q=dns; s=default; b=uTBg0A/5VwFnB8R m+Jt+WRp7jsH1Ia2nXyKlfSjDl1IuHgHnH4bJ64bDBimIsuH2TadGKlJ0vDwe7l2 46n/wr5L71+kqNOzq/oGEq6oBSZViqofrjXc3q38KV+8cfc1HBKvKOgubXFDED27 yLDGy4lJHNNuvCrE6mcF+8MZVp/U= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:from:reply-to:date:message-id :subject:to:cc:content-type; s=default; bh=agmfvteWItbTOQPEH/Tsp NiSrog=; b=IStOYl9cKtGiZJqB2Stv+t6VVjzj0gUAlH25Oj/uEVL54JNrMDAFm A4t1NZghUIgfBtgGn9uR0FPiX2lTTpnqxUseOKTgnxXxvFywUImhP6XAIE0wOGgl 26L/fkVTqPAZG5nSpUZRtE1+1M2OLOzvCs1NZevoc6dy4CUiW2y2wM= Received: (qmail 97102 invoked by alias); 12 Jan 2017 21:29:02 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Delivered-To: mailing list binutils@sourceware.org Received: (qmail 97087 invoked by uid 89); 12 Jan 2017 21:29:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, RP_MATCHES_RCVD, SPF_PASS, UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy=ccoutantgmailcom, ccoutant@gmail.com, sk:ccoutan, U*ccoutant X-HELO: mail-io0-f177.google.com Received: from mail-io0-f177.google.com (HELO mail-io0-f177.google.com) (209.85.223.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 12 Jan 2017 21:28:51 +0000 Received: by mail-io0-f177.google.com with SMTP id j18so29129467ioe.2 for ; Thu, 12 Jan 2017 13:28:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=T6G6dWXC+IloPbPytzhpTZbC+mbPTOF1fDiJydHDGmY=; b=P0GEiYZCnnMFTkEc7DaQKIoZgv3a6ROTUuXvmw5RPEvG3tbYHw76i0MeitgsOL2e4U xFTmBiaXalKDpULlwUguiuldM7kefX2vp0N7Ve/uW86ocgsmxJQ3dRykQNFVzjNrwT9g Um1cXfbWcxWUBWsSiK0r0LkFJujV48V3iY31ITZJ0zR63kuQ0d/xuz+RNfI1i6HQJ1Kb VincSnv76Sf7nK7LqxlYU1Ugq3y6QzSfcx3xGKOUAzmkf4KCekmiJ2M/gquJwcaXEcWg Me9y26jWELIYnokTSOZpH8ue7/CjKpKPMhQOUG2AzQSkXMQsNhvF8nTpB4LDJLQYn9g1 b/lA== X-Gm-Message-State: AIkVDXJkhoKSFi2AVcDU5IOdDjch6QGsOjsg+PpeCXhyjVKZnKz9L7toV+xZVPIVVCVyfEJ8cAxBOYUJz3OQSLh2 X-Received: by 10.107.7.78 with SMTP id 75mr7921588ioh.165.1484256529888; Thu, 12 Jan 2017 13:28:49 -0800 (PST) MIME-Version: 1.0 Received: by 10.64.24.204 with HTTP; Thu, 12 Jan 2017 13:28:49 -0800 (PST) X-Patchwork-Original-From: "Rahul Chaudhry via binutils" From: Rahul Chaudhry via binutils Reply-To: Rahul Chaudhry Date: Thu, 12 Jan 2017 13:28:49 -0800 Message-ID: Subject: [GOLD] Support --icf=safe with -pie for x86_64 To: Binutils Cc: Cary Coutant , Sriraman Tallam --icf=safe folds identical functions only if the functions do not have their address taken. It uses relocation types to distinguish between a function call and taking the address of a function. This works fine for all architectures except x86_64, where the same relocation type (R_X86_64_PC32) may be used for both function call and taking the address of a function (when compiled with -fPIE). Currently Target_x86_64::do_can_check_for_function_pointers returns false if the linker is invoked with -pie, and --icf=safe folds only ctors and dtors. This patch adds improved support for --icf=safe when used with -pie. For the ambiguous case of R_X86_64_PC32 relocation, it checks the instruction opcode to distinguish between function calls and taking the address of a function. OK? * x86_64.cc (Target_x86_64::do_can_check_for_function_pointers): Return true even when building pie binaries. (Target_x86_64::possible_function_pointer_reloc): Check opcode for R_X86_64_PC32 relocations. (Target_x86_64::local_reloc_may_be_function_pointer): Pass extra arguments to local_reloc_may_be_function_pointer. (Target_x86_64::global_reloc_may_be_function_pointer): Likewise. * testsuite/Makefile.am (icf_safe_pie_test): New test case. * testsuite/Makefile.in: Regenerate. * testsuite/icf_safe_pie_test.sh: New shell script. -- Rahul diff --git gold/ChangeLog gold/ChangeLog index 6cc97528a8..05e081186a 100644 --- gold/ChangeLog +++ gold/ChangeLog @@ -1,3 +1,16 @@ +2017-01-12 Rahul Chaudhry + + * x86_64.cc (Target_x86_64::do_can_check_for_function_pointers): + Return true even when building pie binaries. + (Target_x86_64::possible_function_pointer_reloc): Check opcode + for R_X86_64_PC32 relocations. + (Target_x86_64::local_reloc_may_be_function_pointer): Pass + extra arguments to local_reloc_may_be_function_pointer. + (Target_x86_64::global_reloc_may_be_function_pointer): Likewise. + * testsuite/Makefile.am (icf_safe_pie_test): New test case. + * testsuite/Makefile.in: Regenerate. + * testsuite/icf_safe_pie_test.sh: New shell script. + 2017-01-11 Cary Coutant PR gold/21040 diff --git gold/testsuite/Makefile.am gold/testsuite/Makefile.am index d9480abb42..d0803d251a 100644 --- gold/testsuite/Makefile.am +++ gold/testsuite/Makefile.am @@ -274,6 +274,20 @@ icf_safe_test_1.stdout: icf_safe_test icf_safe_test_2.stdout: icf_safe_test $(TEST_READELF) -h $< > $@ +check_SCRIPTS += icf_safe_pie_test.sh +check_DATA += icf_safe_pie_test_1.stdout icf_safe_pie_test_2.stdout icf_safe_pie_test.map +MOSTLYCLEANFILES += icf_safe_pie_test icf_safe_pie_test.map +icf_safe_pie_test.o: icf_safe_test.cc + $(CXXCOMPILE) -O0 -c -ffunction-sections -fPIE -g -o $@ $< +icf_safe_pie_test: icf_safe_pie_test.o gcctestdir/ld + $(CXXLINK) -o icf_safe_pie_test -Bgcctestdir/ -Wl,--icf=safe,-Map,icf_safe_pie_test.map icf_safe_pie_test.o -pie +icf_safe_pie_test.map: icf_safe_pie_test + @touch icf_safe_pie_test.map +icf_safe_pie_test_1.stdout: icf_safe_pie_test + $(TEST_NM) $< > $@ +icf_safe_pie_test_2.stdout: icf_safe_pie_test + $(TEST_READELF) -h $< > $@ + check_SCRIPTS += icf_safe_so_test.sh check_DATA += icf_safe_so_test_1.stdout icf_safe_so_test_2.stdout icf_safe_so_test.map MOSTLYCLEANFILES += icf_safe_so_test icf_safe_so_test.map diff --git gold/testsuite/Makefile.in gold/testsuite/Makefile.in index 2c67bf5fe2..133e733cf1 100644 --- gold/testsuite/Makefile.in +++ gold/testsuite/Makefile.in @@ -82,6 +82,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_test.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_keep_unique_test.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_test.sh \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ final_layout.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ text_section_grouping.sh \ @@ -103,6 +104,9 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_test_1.stdout \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_test_2.stdout \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_test.map \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test_1.stdout \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test_2.stdout \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test.map \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test_1.stdout \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test_2.stdout \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test.map \ @@ -125,6 +129,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_test icf_test.map \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_keep_unique_test \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_test icf_safe_test.map \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test.map \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test.map \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ final_layout \ @@ -5093,6 +5099,8 @@ icf_keep_unique_test.sh.log: icf_keep_unique_test.sh @p='icf_keep_unique_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) icf_safe_test.sh.log: icf_safe_test.sh @p='icf_safe_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) +icf_safe_pie_test.sh.log: icf_safe_pie_test.sh + @p='icf_safe_pie_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) icf_safe_so_test.sh.log: icf_safe_so_test.sh @p='icf_safe_so_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) final_layout.sh.log: final_layout.sh @@ -5910,6 +5918,16 @@ uninstall-am: @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_NM) $< > $@ @GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_test_2.stdout: icf_safe_test @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) -h $< > $@ +@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_pie_test.o: icf_safe_test.cc +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -c -ffunction-sections -fPIE -g -o $@ $< +@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_pie_test: icf_safe_pie_test.o gcctestdir/ld +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -o icf_safe_pie_test -Bgcctestdir/ -Wl,--icf=safe,-Map,icf_safe_pie_test.map icf_safe_pie_test.o -pie +@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_pie_test.map: icf_safe_pie_test +@GCC_TRUE@@NATIVE_LINKER_TRUE@ @touch icf_safe_pie_test.map +@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_pie_test_1.stdout: icf_safe_pie_test +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_NM) $< > $@ +@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_pie_test_2.stdout: icf_safe_pie_test +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) -h $< > $@ @GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_so_test.o: icf_safe_so_test.cc @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -c -ffunction-sections -fPIC -g -o $@ $< @GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_so_test: icf_safe_so_test.o gcctestdir/ld diff --git gold/testsuite/icf_safe_pie_test.sh gold/testsuite/icf_safe_pie_test.sh new file mode 100755 index 0000000000..f91a80c0b9 --- /dev/null +++ gold/testsuite/icf_safe_pie_test.sh @@ -0,0 +1,76 @@ +#!/bin/sh + +# icf_safe_pie_test.sh -- test --icf=safe -pie + +# Copyright (C) 2009-2017 Free Software Foundation, Inc. +# Written by Sriraman Tallam . +# Modified by Rahul Chaudhry . + +# This file is part of gold. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, +# MA 02110-1301, USA. + +# The goal of this program is to verify if --icf=safe works with +# -pie as expected. File icf_safe_test.cc is in this test. This +# program checks if only ctors and dtors are folded, except for +# the architectures which use relocation types and instruction +# opcodes to detect if function pointers are taken. + +set -e + +check_nofold() +{ + func_addr_1=`grep $2 $1 | awk '{print $1}'` + func_addr_2=`grep $3 $1 | awk '{print $1}'` + if [ $func_addr_1 = $func_addr_2 ] + then + echo "Safe Identical Code Folding folded" $2 "and" $3 + exit 1 + fi +} + +check_fold() +{ + awk " +BEGIN { discard = 0; } +/^Discarded input/ { discard = 1; } +/^Memory map/ { discard = 0; } +/.*\\.text\\..*($2|$3).*/ { act[discard] = act[discard] \" \" \$0; } +END { + # printf \"kept\" act[0] \"\\nfolded\" act[1] \"\\n\"; + if (length(act[0]) == 0 || length(act[1]) == 0) + { + printf \"Safe Identical Code Folding did not fold $2 and $3\\n\" + exit 1; + } + }" $1 +} + +arch_specific_safe_fold() +{ + if grep -q -e "Advanced Micro Devices X86-64" -e "Intel 80386" -e "ARM" -e "TILE" -e "PowerPC" -e "AArch64" -e "IBM S/390" $2; + then + check_fold $3 $4 $5 + else + check_nofold $1 $4 $5 + fi +} + +arch_specific_safe_fold icf_safe_pie_test_1.stdout icf_safe_pie_test_2.stdout \ + icf_safe_pie_test.map "kept_func_1" "kept_func_2" +check_fold icf_safe_pie_test.map "_ZN1AD2Ev" "_ZN1AC2Ev" +check_nofold icf_safe_pie_test_1.stdout "kept_func_3" "kept_func_1" +check_nofold icf_safe_pie_test_1.stdout "kept_func_3" "kept_func_2" diff --git gold/x86_64.cc gold/x86_64.cc index ffa876116a..8360033d8c 100644 --- gold/x86_64.cc +++ gold/x86_64.cc @@ -729,10 +729,13 @@ class Target_x86_64 : public Sized_target // and global_reloc_may_be_function_pointer) // if a function's pointer is taken. ICF uses this in safe mode to only // fold those functions whose pointer is defintely not taken. For x86_64 - // pie binaries, safe ICF cannot be done by looking at relocation types. + // pie binaries, safe ICF cannot be done by looking at only relocation + // types, and for certain cases (e.g. R_X86_64_PC32), the instruction + // opcode is checked as well to distinguish a function call from taking + // a function's pointer. bool do_can_check_for_function_pointers() const - { return !parameters->options().pie(); } + { return true; } // Return the base for a DW_EH_PE_datarel encoding. uint64_t @@ -924,7 +927,10 @@ class Target_x86_64 : public Sized_target check_non_pic(Relobj*, unsigned int r_type, Symbol*); inline bool - possible_function_pointer_reloc(unsigned int r_type); + possible_function_pointer_reloc(Sized_relobj_file* src_obj, + unsigned int src_indx, + unsigned int r_offset, + unsigned int r_type); bool reloc_needs_plt_for_ifunc(Sized_relobj_file*, @@ -3274,7 +3280,11 @@ Target_x86_64::Scan::unsupported_reloc_global( // Returns true if this relocation type could be that of a function pointer. template inline bool -Target_x86_64::Scan::possible_function_pointer_reloc(unsigned int r_type) +Target_x86_64::Scan::possible_function_pointer_reloc( + Sized_relobj_file* src_obj, + unsigned int src_indx, + unsigned int r_offset, + unsigned int r_type) { switch (r_type) { @@ -3293,6 +3303,36 @@ Target_x86_64::Scan::possible_function_pointer_reloc(unsigned int r_type) { return true; } + case elfcpp::R_X86_64_PC32: + { + // This relocation may be used both for function calls and + // for taking address of a function. We distinguish between + // them by checking the opcodes. + section_size_type stype; + const unsigned char* view = src_obj->section_contents(src_indx, + &stype, + true); + + // call + if (r_offset >= 1 + && view[r_offset - 1] == 0xe8) + return false; + + // jmp + if (r_offset >= 1 + && view[r_offset - 1] == 0xe9) + return false; + + // jo/jno/jb/jnb/je/jne/jna/ja/js/jns/jp/jnp/jl/jge/jle/jg + if (r_offset >= 2 + && view[r_offset - 2] == 0x0f + && view[r_offset - 1] >= 0x80 + && view[r_offset - 1] <= 0x8f) + return false; + + // Be conservative and treat all others as function pointers. + return true; + } } return false; } @@ -3307,18 +3347,21 @@ Target_x86_64::Scan::local_reloc_may_be_function_pointer( Symbol_table* , Layout* , Target_x86_64* , - Sized_relobj_file* , - unsigned int , + Sized_relobj_file* src_obj, + unsigned int src_indx, Output_section* , - const elfcpp::Rela& , + const elfcpp::Rela& reloc, unsigned int r_type, const elfcpp::Sym&) { // When building a shared library, do not fold any local symbols as it is // not possible to distinguish pointer taken versus a call by looking at // the relocation types. - return (parameters->options().shared() - || possible_function_pointer_reloc(r_type)); + if (parameters->options().shared()) + return true; + + return possible_function_pointer_reloc(src_obj, src_indx, + reloc.get_r_offset(), r_type); } // For safe ICF, scan a relocation for a global symbol to check if it @@ -3331,20 +3374,23 @@ Target_x86_64::Scan::global_reloc_may_be_function_pointer( Symbol_table*, Layout* , Target_x86_64* , - Sized_relobj_file* , - unsigned int , + Sized_relobj_file* src_obj, + unsigned int src_indx, Output_section* , - const elfcpp::Rela& , + const elfcpp::Rela& reloc, unsigned int r_type, Symbol* gsym) { // When building a shared library, do not fold symbols whose visibility // is hidden, internal or protected. - return ((parameters->options().shared() - && (gsym->visibility() == elfcpp::STV_INTERNAL - || gsym->visibility() == elfcpp::STV_PROTECTED - || gsym->visibility() == elfcpp::STV_HIDDEN)) - || possible_function_pointer_reloc(r_type)); + if (parameters->options().shared() + && (gsym->visibility() == elfcpp::STV_INTERNAL + || gsym->visibility() == elfcpp::STV_PROTECTED + || gsym->visibility() == elfcpp::STV_HIDDEN)) + return true; + + return possible_function_pointer_reloc(src_obj, src_indx, + reloc.get_r_offset(), r_type); } // Scan a relocation for a global symbol.