From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x533.google.com (mail-ed1-x533.google.com [IPv6:2a00:1450:4864:20::533]) by sourceware.org (Postfix) with ESMTPS id BADB43858036 for ; Thu, 30 Jun 2022 05:31:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BADB43858036 Received: by mail-ed1-x533.google.com with SMTP id eq6so24990408edb.6 for ; Wed, 29 Jun 2022 22:31:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:content-transfer-encoding:from:mime-version :subject:date:message-id:references:cc:in-reply-to:to; bh=/E+Y6aonPYIsNcv22fEXXjfaPKJW+SrRU7v9jI0TY+w=; b=jBPiZx/C4ejt1a49p+7DuaFDts7HkpsOQ/2XAruqNpdNNlO6wYo6gBd2/ZGjbcq7P9 gmZbn8GpCDR44pukZQkVo4GXmMgsRQku6Te6pcVLXJQs+h6MsgqpteolK1J1TQtpfEBu rTE+VR5839DTTdbEN7sqbUZmcM3sDc1DT72qf0co+4azhqhchJzkVX64eNF+jxhu7M4P 1YyHOt2g3Mhj7g7p+cELoqbebTAZnuNlm7CYbuATOpt8QGgOmkNrueZJG4cI0MPu/zAw s6Gxb8U9AcMJDnYoZpRXD+6ktAU19d7YKz9uGUY21Gzl5/L2pxddyJPYqn/YL915qBXS wUTA== X-Gm-Message-State: AJIora/N+BVr67GZLghOt/qwtYD78b/UujAJaLaSH49YgdO3pQudBWZV fKFZ5YYuDptl0y9sRlAV1QxYmf6Cc+M= X-Google-Smtp-Source: AGRyM1vX6Z8fZKrccE+d9bF1xhvxEZLIq/FqzDH3142t2xh+BmctaJB09RRdF7mXwPIT/emExycqKA== X-Received: by 2002:a05:6402:2c4:b0:435:8ce0:aef8 with SMTP id b4-20020a05640202c400b004358ce0aef8mr9212518edx.140.1656567086295; Wed, 29 Jun 2022 22:31:26 -0700 (PDT) Received: from smtpclient.apple (dynamic-095-117-120-011.95.117.pool.telefonica.de. [95.117.120.11]) by smtp.gmail.com with ESMTPSA id m9-20020a509989000000b004355d27799fsm12742699edb.96.2022.06.29.22.31.25 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Jun 2022 22:31:25 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable From: Richard Biener Mime-Version: 1.0 (1.0) Subject: Re: lto: Fix option merging [PR106129] Date: Thu, 30 Jun 2022 07:31:25 +0200 Message-Id: <8E570ED4-A0C7-4645-BB4B-112F906CAEBC@gmail.com> References: Cc: gcc-patches@gcc.gnu.org In-Reply-To: To: Joseph Myers X-Mailer: iPhone Mail (19F77) X-Spam-Status: No, score=-9.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: Thu, 30 Jun 2022 05:31:29 -0000 > Am 29.06.2022 um 22:44 schrieb Joseph Myers : >=20 > =EF=BB=BFThe LTO merging of options from different input files was broken b= y: >=20 > commit 227a2ecf663d69972b851f51f1934d18927b62cd > Author: Martin Liska > Date: Fri Mar 12 11:53:47 2021 +0100 >=20 > lto-wrapper: Use vec data type. >=20 > Previously, find_and_merge_options would merge options it read into > those in *opts. After this commit, options in *opts on entry to > find_and_merge_options are ignored; the only merging that takes place > is between multiple sets of options in the same input file that are > read in the same call to this function (not sure how that case can > occur at all). The effects include, for example, that if some objects > are built with PIC enabled and others with it disabled, and the last > LTO object processed has PIC enabled, the choice of PIC for the last > object will result in the whole program being built as PIC, when the > merging logic is intended to ensure that a mixture of PIC and non-PIC > objects results in the whole program being built as non-PIC. >=20 > Fix this with an extra argument to find_and_merge_options to determine > whether merging should take place. This shows up a second issue with > that commit (which I think wasn't actually intended to change code > semantics at all): once merging is enabled again, the check for > -Xassembler options became an infinite loop in the case where both > inputs had -Xassembler options, with the same first option, so fix > that loop to restore the previous semantics. >=20 > Note that I'm not sure how LTO option merging might be tested in the > testsuite (clearly there wasn't sufficient, if any, coverage to detect > these bugs). >=20 > Bootstrapped with no regressions for x86_64-pc-linux-gnu. OK to > commit? >=20 Ok. Thanks, Richard=20 > PR lto/106129 > * lto-wrapper.cc (find_option): Add argument start. > (merge_and_complain): Loop over existing_opt_index and > existing_opt2_index for Xassembler check. Update calls to > find_option. > (find_and_merge_options): Add argument first to determine whether > to merge options with those passed in *opts. > (run_gcc): Update calls to find_and_merge_options. >=20 > diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc > index 26e06e7..795ab74 100644 > --- a/gcc/lto-wrapper.cc > +++ b/gcc/lto-wrapper.cc > @@ -170,13 +170,14 @@ get_options_from_collect_gcc_options (const char *co= llect_gcc, > return decoded; > } >=20 > -/* Find option in OPTIONS based on OPT_INDEX. -1 value is returned > - if the option is not present. */ > +/* Find option in OPTIONS based on OPT_INDEX, starting at START. -1 > + value is returned if the option is not present. */ >=20 > static int > -find_option (vec &options, size_t opt_index) > +find_option (vec &options, size_t opt_index, > + unsigned start =3D 0) > { > - for (unsigned i =3D 0; i < options.length (); ++i) > + for (unsigned i =3D start; i < options.length (); ++i) > if (options[i].opt_index =3D=3D opt_index) > return i; >=20 > @@ -575,13 +576,16 @@ merge_and_complain (vec &decoded_= options, > else > j++; >=20 > + int existing_opt_index, existing_opt2_index; > if (!xassembler_options_error) > - for (i =3D j =3D 0; ; i++, j++) > + for (existing_opt_index =3D existing_opt2_index =3D 0; ; > + existing_opt_index++, existing_opt2_index++) > { > - int existing_opt_index > - =3D find_option (decoded_options, OPT_Xassembler); > - int existing_opt2_index > - =3D find_option (fdecoded_options, OPT_Xassembler); > + existing_opt_index > + =3D find_option (decoded_options, OPT_Xassembler, existing_opt_inde= x); > + existing_opt2_index > + =3D find_option (fdecoded_options, OPT_Xassembler, > + existing_opt2_index); >=20 > cl_decoded_option *existing_opt =3D NULL; > cl_decoded_option *existing_opt2 =3D NULL; > @@ -1100,7 +1104,7 @@ find_crtoffloadtable (int save_temps, const char *du= mppfx) >=20 > static bool > find_and_merge_options (int fd, off_t file_offset, const char *prefix, > - vec decoded_cl_options, > + vec decoded_cl_options, bool first, > vec *opts, const char *collect_gcc) > { > off_t offset, length; > @@ -1110,6 +1114,9 @@ find_and_merge_options (int fd, off_t file_offset, c= onst char *prefix, > int err; > vec fdecoded_options; >=20 > + if (!first) > + fdecoded_options =3D *opts; > + > simple_object_read *sobj; > sobj =3D simple_object_start_read (fd, file_offset, "__GNU_LTO", > &errmsg, &err); > @@ -1130,7 +1137,6 @@ find_and_merge_options (int fd, off_t file_offset, c= onst char *prefix, > data =3D (char *)xmalloc (length); > read (fd, data, length); > fopts =3D data; > - bool first =3D true; > do > { > vec f2decoded_options > @@ -1417,8 +1423,10 @@ run_gcc (unsigned argc, char *argv[]) > int auto_parallel =3D 0; > bool no_partition =3D false; > const char *jobserver_error =3D NULL; > + bool fdecoded_options_first =3D true; > vec fdecoded_options; > fdecoded_options.create (16); > + bool offload_fdecoded_options_first =3D true; > vec offload_fdecoded_options =3D vNULL; > struct obstack argv_obstack; > int new_head_argc; > @@ -1510,11 +1518,13 @@ run_gcc (unsigned argc, char *argv[]) > } >=20 > if (find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX= , > - decoded_options, &fdecoded_options, > + decoded_options, fdecoded_options_first, > + &fdecoded_options, > collect_gcc)) > { > have_lto =3D true; > ltoobj_argv[ltoobj_argc++] =3D argv[i]; > + fdecoded_options_first =3D false; > } > close (fd); > } > @@ -1773,9 +1783,12 @@ cont1: > fatal_error (input_location, "cannot open %s: %m", filename); > if (!find_and_merge_options (fd, file_offset, > OFFLOAD_SECTION_NAME_PREFIX, > - decoded_options, &offload_fdecoded_options, > + decoded_options, > + offload_fdecoded_options_first, > + &offload_fdecoded_options, > collect_gcc)) > fatal_error (input_location, "cannot read %s: %m", filename); > + offload_fdecoded_options_first =3D false; > close (fd); > if (filename !=3D offload_argv[i]) > XDELETEVEC (filename); >=20 > --=20 > Joseph S. Myers > joseph@codesourcery.com