public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Renlin Li <renlin.li@foss.arm.com>
To: Christophe Lyon <christophe.lyon@linaro.org>
Cc: kugan <kugan.vivekanandarajah@linaro.org>,
	"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	Kyrill Tkachov <kyrylo.tkachov@arm.com>,
	Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>
Subject: Re: [PATCH][ARM]Use different startfile and endfile for elf target when generating shared object.
Date: Fri, 13 Jan 2017 10:22:00 -0000	[thread overview]
Message-ID: <5878AA6D.9000603@foss.arm.com> (raw)
In-Reply-To: <CAKdteOY6dQE4DF-1+wT1RgKwMLkypKnFiTctZ0w5Zid7L=AhhA@mail.gmail.com>

Hi Christophe,

Thanks for testing the patch!
I check the test case gcc.dg/lto/pr54709, it seems the test case is not properly written.

It add extra ld option -shared without checking the target support for that. After the 
change, this compilation will fail as a regression.
IIUC, '-shared' option is required, it should be gated with corresponding target selector.

"g++.dg/ipa/devirt-28a.C" now is skipped because of the target selector there.
// { dg-do link { target { { gld && fpic } && shared } } }

perhaps "gcc.dg/lto/pr54709" should do similar things like this:
// { dg-do link { target { shared } } }


As far as I know, with different cpu/arch configurations, different relocations are 
generated in the library, some of the relocations are not allowed to be used in shared
object.

With -march=armv7-a (and the --with-cpu=cortex-a9 option you mentioned), the linking stage 
of the test will fail because of this error:
"relocation XXX against external symbol `YYY' can not be used when making a shared object"
for instance: crtbegin.o: relocation R_ARM_MOVW_ABS_NC against `a local symbol` can not be 
used when making a shared object; recompile with -fPIC

If you are luck enough, for example with arm7tdmi cpu, no such relocation is generated in 
startup files. The "shared" target support check will pass for simple and naive code.
"--with-cpu=cortex-m3" should be this case. But the test cases which require shared object 
support will fail.


So this "shared" target checking mechanism is not reliable. The patch is to change this.



Regards,
Renlin



On 13/01/17 08:48, Christophe Lyon wrote:
> Hi Renlin,
>
>
> On 12 January 2017 at 16:50, Renlin Li <renlin.li@foss.arm.com> wrote:
>> Hi Kugan,
>>
>> some of the targets do include pie, and use the same crtbegin file as shared
>> object.
>> For example, alpha/elf.h
>>
>> And there are targets which don't do that,
>> For example, sh/elf.h
>>
>> Most of the elf target seem only consider the simple case.
>>
>> The purpose of this patch is to make it possible to correctly check whether
>> current toolchain supports shared object.
>>
>> Current dejegnu target selector "shared" tries to compile a simple source
>> code to with "-shared -fpic" options to check whether "-shared" is
>> supported.
>>
>> For arm baremetal targets with, this is not sufficient.
>>
>> arm-none-eabi is built with multilib. When running this testcase, if it's
>> compiled with "-march=armv7-a".
>> The crtbegin.o for this architecture version contains relocations which
>> cannot be used in shared object.
>> This test will fail.
>>
>> if no cpu or architecture is specified, the default cpu will be arm7tdmi.
>> The test will pass as crtbegin.o for this version doesn't contains any
>> relocations
>> only allowed in shared object.
>>
>> To make this "shared" target selector work for arm baremetal toolchain. The
>> correct way is to use different startup file for shared and non-shared
>> toolchain.
>>
>> If the toolchain doesn't support "-shared" option, and someone attempts to
>> use it
>> to create shared object, it will complaint that "crtbeginS.o" cannot not be
>> found.
>>
>
> I have run validations with your patch, and noticed regressions on arm-none-eabi
> when using default cpu or --with-cpu=cortex-m3:
>
>    - PASS now FAIL             [PASS => FAIL]:
>
>    gcc.dg/lto/pr54709 c_lto_pr54709_0.o-c_lto_pr54709_1.o link,  -fPIC
> -fvisibility=hidden -flto
>    gcc.dg/lto/pr61526 c_lto_pr61526_0.o-c_lto_pr61526_1.o link,  -fPIC
> -flto -flto-partition=1to1
>    gcc.dg/lto/pr64415 c_lto_pr64415_0.o-c_lto_pr64415_1.o link,  -O -flto -fpic
>
> on the same configurations, I've noticed these improvements:
>
>    g++.dg/ipa/devirt-28a.C  -std=gnu++11 (test for excess errors)
>    g++.dg/ipa/devirt-28a.C  -std=gnu++14 (test for excess errors)
>    g++.dg/ipa/devirt-28a.C  -std=gnu++98 (test for excess errors)
> are now unsupported rather than fail.
>
> Why is it different when the toolchain is configured --with-cpu=cortex-a9
> for instance? Are the tests involving -shared already skipped in this case?
>
>> A full history of discussion is here:
>> https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00322.html
>>
>>
>> Regards,
>> Renlin
>>
>>
>>
>> On 12/01/17 11:47, kugan wrote:
>>>
>>> Hi,
>>>
>>> On 16/06/16 21:04, Renlin Li wrote:
>>>>
>>>>   /* Now we define the strings used to build the spec file.  */
>>>> -#define UNKNOWN_ELF_STARTFILE_SPEC    " crti%O%s crtbegin%O%s crt0%O%s"
>>>> +#define UNKNOWN_ELF_STARTFILE_SPEC    \
>>>> +  "crti%O%s \
>>>> +  %{!shared:crtbegin%O%s} %{shared:crtbeginS%O%s} \
>>>> +  crt0%O%s"
>>>
>>>
>>> Some targets seems to use shared|pie. When you change it, shouldn't it
>>> also include for pie?
>>>
>>> Thanks,
>>> Kugan

  reply	other threads:[~2017-01-13 10:22 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-16 11:04 Renlin Li
2016-12-14 15:34 ` [PING][PATCH][ARM]Use " Renlin Li
2017-01-12 11:24   ` Renlin Li
2017-06-07 16:41   ` Renlin Li
2017-01-12 11:47 ` [PATCH][ARM]Use " kugan
2017-01-12 15:50   ` Renlin Li
2017-01-13  8:48     ` Christophe Lyon
2017-01-13 10:22       ` Renlin Li [this message]
2017-01-13 11:14         ` Christophe Lyon
2017-01-13 12:26           ` Renlin Li
2017-01-13 12:45             ` Christophe Lyon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5878AA6D.9000603@foss.arm.com \
    --to=renlin.li@foss.arm.com \
    --cc=Ramana.Radhakrishnan@arm.com \
    --cc=christophe.lyon@linaro.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=kugan.vivekanandarajah@linaro.org \
    --cc=kyrylo.tkachov@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).