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...@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 via email to