[PATCH 0/2] ARC Moving to @pcl relocations

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 0/2] ARC Moving to @pcl relocations

Andrew Burgess
This is a slightly odd series of 2 patches.  The two patches are
actually alternative solutions to the same problem.  I'm keen to see
one of these merged, but I don't know which method would be preferred.

This commit aims to address an issue that was introduced / mentioned
in commit 20554a78a9bba278c6b9cbbb4cfc5bde3772c56d (ARC:
Conditionalise certain relocations as provided by TLS tools only).

The problem is that not all historic versions of binutils have
supported the @pcl relocation type.  This problem is compounded by the
fact that the arithmetic construct that was previously used to
synthesise an @pcl like behaviour, does not work on recent versions of
binutils.

In the commit 20554a78a code was added that selects between the new
style @pcl relocations, and the old style arithmetic construct,
however, this selection is done based on whether the uClibc user has
configured with native threads or not.

Of course, a uClibc user could choose to use a modern version of
binutils AND configure without native thread support, in which case
uClibc will not compile.

I have two proposed solutions.  In patch 1/2 I simply drop support for
the older versions of binutils in favour of the new @pcl relocation
type.  This feels the cleanest solution, but I don't know how strongly
the uClibc(-ng) community feels about maintaining compatibility for
older versions of the tools.

Given that I anticipated push back against the first patch I took a
look at how I might maintain compatibility.  It turns out to be pretty
easy, and that is patch 2/2.  In this patch I drew inspiration from
similar examples in the Rules.mak file to check if the toolchain
supports @pcl relocations or not.  With this done we have a new define
based on the specific toolchain feature being supported or not, which
can then be used to conditionalise the code.

Would you consider merging one of these patches?

Thanks,
Andrew
_______________________________________________
uClibc mailing list
[hidden email]
http://lists.busybox.net/mailman/listinfo/uclibc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/2] ARC: Update relocation syntax for old-thread model code

Andrew Burgess
This commit reverses a change introduced in commit 20554a78a9bba that
split some of the ARC code into two based on whether uClibc was
configured with native threads or not.

The native thread code was updated to use the relocation syntax of
modern binutils, while the non-native code path used a syntax only
accepted in older versions of binutils.

The problem with this is that the choice of old binutils or not is
orthogonal to the choice of native threads or not, and so, inevitably a
user with a recent version of binutils can make the choice to configure
uClibc with non-native thread support, and run into code that will not
assemble.

The solution is either to abandon support for the old tools completely,
or to add a new compile time flag for ARC that is set when the version
of binutils being used is old; this new flag would allow the old
relocation structure to be selected.

In this commit I have simply dropped support for older versions of the
tools.
---
 ldso/ldso/arc/dl-startup.h | 7 -------
 ldso/ldso/arc/dl-sysdep.h  | 2 +-
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/ldso/ldso/arc/dl-startup.h b/ldso/ldso/arc/dl-startup.h
index ef89b53..664b860 100644
--- a/ldso/ldso/arc/dl-startup.h
+++ b/ldso/ldso/arc/dl-startup.h
@@ -34,15 +34,8 @@ __asm__(
     "   ; skip the extra args calc by dl_start()                \n"
     "   ld_s    r1, [sp]       ; orig argc from aux-vec Tbl     \n"
 
-#ifdef __UCLIBC_HAS_THREADS_NATIVE__
     "   ld      r12, [pcl, _dl_skip_args@pcl]                   \n"
-
     "   add     r2, pcl, _dl_fini@pcl       ; finalizer         \n"
-#else
-    "   add     r12, pcl, _dl_skip_args-.+(.&2)                 \n"
-    "   ld      r12, [r12]                                      \n"
-    "   add     r2, pcl, _dl_fini-.+(.&2)   ; finalizer         \n"
-#endif
 
     "   add2    sp, sp, r12    ; discard argv entries from stack\n"
     "   sub_s   r1, r1, r12    ; adjusted argc, on stack        \n"
diff --git a/ldso/ldso/arc/dl-sysdep.h b/ldso/ldso/arc/dl-sysdep.h
index caece99..aadf624 100644
--- a/ldso/ldso/arc/dl-sysdep.h
+++ b/ldso/ldso/arc/dl-sysdep.h
@@ -154,7 +154,7 @@ static __always_inline Elf32_Addr elf_machine_load_address(void)
  Elf32_Addr addr, tmp;
  __asm__ (
         "ld  %1, [pcl, _dl_start@gotpc] ;build addr of _dl_start   \n"
-        "add %0, pcl, _dl_start-.+(.&2) ;runtime addr of _dl_start \n"
+        "add %0, pcl, _dl_start@pcl     ;runtime addr of _dl_start \n"
         "sub %0, %0, %1                 ;delta                     \n"
          : "=&r" (addr), "=r"(tmp)
      );
--
2.5.1

_______________________________________________
uClibc mailing list
[hidden email]
http://lists.busybox.net/mailman/listinfo/uclibc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/2] ARC: Add new compiler define to indicate @pcl relocation support

Andrew Burgess
In reply to this post by Andrew Burgess
Some old versions of binutils did not support @pcl relocations.  This
commit adds a new flag to the uClibc configuration system that detects
if the toolchain supports @pcl relocations or not.

If this relocation is supported then the define ARC_HAS_AT_PCL_RELOC
will be passed to the compiler, which is then used in the arc ldso to
choose between generating old or new style code.

This commit addresses and issue that was worked around in commit
181d410ad00cddd1d6c9f4835e129136b74c5187 (ARC: Conditionalise certain
relocations as provided by TLS tools only).  In this commit the choice
between old or new style relocations was made based on whether uClibc
was configured with native threads or not.  The problem is that a user
of a new toolchain might choose to configure without native threads.
---
 Rules.mak                  | 2 ++
 ldso/ldso/arc/dl-startup.h | 2 +-
 ldso/ldso/arc/dl-sysdep.h  | 4 ++++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Rules.mak b/Rules.mak
index 3c80016..04ff02f 100644
--- a/Rules.mak
+++ b/Rules.mak
@@ -507,9 +507,11 @@ ifeq ($(TARGET_ARCH),c6x)
 endif
 
 ifeq ($(TARGET_ARCH),arc)
+ ARC_HAS_AT_PCL_RELOC:=$(shell echo -e "\t.text\n\tadd r0,pcl,_symbol@pcl" | $(CC) -c -x assembler -o /dev/null -  2> /dev/null && echo -n y || echo -n n)
  CPU_CFLAGS-y += -mlock -mswape
  CPU_CFLAGS-$(CONFIG_ARC_CPU_700) += -mA7
  CPU_CFLAGS-$(CONFIG_ARC_CPU_HS) += -mcpu=archs
+ CPU_CFLAGS-$(ARC_HAS_AT_PCL_RELOC) += -DARC_HAS_AT_PCL_RELOC
  CPU_LDFLAGS-y += $(CPU_CFLAGS) -marclinux
 endif
 
diff --git a/ldso/ldso/arc/dl-startup.h b/ldso/ldso/arc/dl-startup.h
index ef89b53..80ffd79 100644
--- a/ldso/ldso/arc/dl-startup.h
+++ b/ldso/ldso/arc/dl-startup.h
@@ -34,7 +34,7 @@ __asm__(
     "   ; skip the extra args calc by dl_start()                \n"
     "   ld_s    r1, [sp]       ; orig argc from aux-vec Tbl     \n"
 
-#ifdef __UCLIBC_HAS_THREADS_NATIVE__
+#ifdef ARC_HAS_AT_PCL_RELOC
     "   ld      r12, [pcl, _dl_skip_args@pcl]                   \n"
 
     "   add     r2, pcl, _dl_fini@pcl       ; finalizer         \n"
diff --git a/ldso/ldso/arc/dl-sysdep.h b/ldso/ldso/arc/dl-sysdep.h
index caece99..27463f0 100644
--- a/ldso/ldso/arc/dl-sysdep.h
+++ b/ldso/ldso/arc/dl-sysdep.h
@@ -154,7 +154,11 @@ static __always_inline Elf32_Addr elf_machine_load_address(void)
  Elf32_Addr addr, tmp;
  __asm__ (
         "ld  %1, [pcl, _dl_start@gotpc] ;build addr of _dl_start   \n"
+#ifdef ARC_HAS_AT_PCL_RELOC
+        "add %0, pcl, _dl_start@pcl     ;runtime addr of _dl_start \n"
+#else
         "add %0, pcl, _dl_start-.+(.&2) ;runtime addr of _dl_start \n"
+#endif /* ARC_HAS_AT_PCL_RELOC */
         "sub %0, %0, %1                 ;delta                     \n"
          : "=&r" (addr), "=r"(tmp)
      );
--
2.5.1

_______________________________________________
uClibc mailing list
[hidden email]
http://lists.busybox.net/mailman/listinfo/uclibc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 0/2] ARC Moving to @pcl relocations

Vineet Gupta-5
In reply to this post by Andrew Burgess
Hi Andrew,

On 07/28/2016 11:20 AM, Andrew Burgess wrote:

> This is a slightly odd series of 2 patches.  The two patches are
> actually alternative solutions to the same problem.  I'm keen to see
> one of these merged, but I don't know which method would be preferred.
>
> This commit aims to address an issue that was introduced / mentioned
> in commit 20554a78a9bba278c6b9cbbb4cfc5bde3772c56d (ARC:
> Conditionalise certain relocations as provided by TLS tools only).
>
> The problem is that not all historic versions of binutils have
> supported the @pcl relocation type.  This problem is compounded by the
> fact that the arithmetic construct that was previously used to
> synthesise an @pcl like behaviour, does not work on recent versions of
> binutils.
>
> In the commit 20554a78a code was added that selects between the new
> style @pcl relocations, and the old style arithmetic construct,
> however, this selection is done based on whether the uClibc user has
> configured with native threads or not.

Right - the idea at the time was @pcl was added to tools for TLS/NPTL support and
thus uClibc NPTL loosely implied that your tools will have that support - but this
was not the ideal choice I agree.

> Of course, a uClibc user could choose to use a modern version of
> binutils AND configure without native thread support, in which case
> uClibc will not compile.
>
> I have two proposed solutions.  In patch 1/2 I simply drop support for
> the older versions of binutils in favour of the new @pcl relocation
> type.  This feels the cleanest solution, but I don't know how strongly
> the uClibc(-ng) community feels about maintaining compatibility for
> older versions of the tools.

So this was reported recently by Waldemar's build service too.

http://mailman.uclibc-ng.org/pipermail/devel/2016-July/001072.html

And we have an exact same patch floated internally which Vlad sent out earlier
this week

> Given that I anticipated push back against the first patch I took a
> look at how I might maintain compatibility.

Not really - while indeed 1/2 opens up a non-compatibilty window, there is less
likelihood people will mix-n-match different baselines of uclibc and gcc/gas. So
Vlad's patch does exactly that.

> It turns out to be pretty
> easy, and that is patch 2/2.  In this patch I drew inspiration from
> similar examples in the Rules.mak file to check if the toolchain
> supports @pcl relocations or not.  With this done we have a new define
> based on the specific toolchain feature being supported or not, which
> can then be used to conditionalise the code.

Indeed your 2/2 seems to be the most "past-proof" code change. So I would think it
is indeed better and is something I should have done in the first place.

@Alexey, @Vlad what say you ?

-Vineet

> Would you consider merging one of these patches?
>
> Thanks,
> Andrew
>

_______________________________________________
uClibc mailing list
[hidden email]
http://lists.busybox.net/mailman/listinfo/uclibc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 0/2] ARC Moving to @pcl relocations

Bernhard Reutner-Fischer
On July 28, 2016 8:50:41 PM GMT+02:00, Vineet Gupta <[hidden email]> wrote:

>Hi Andrew,
>
>On 07/28/2016 11:20 AM, Andrew Burgess wrote:
>> This is a slightly odd series of 2 patches.  The two patches are
>> actually alternative solutions to the same problem.  I'm keen to see
>> one of these merged, but I don't know which method would be
>preferred.
>>
>> This commit aims to address an issue that was introduced / mentioned
>> in commit 20554a78a9bba278c6b9cbbb4cfc5bde3772c56d (ARC:
>> Conditionalise certain relocations as provided by TLS tools only).
>>
>> The problem is that not all historic versions of binutils have
>> supported the @pcl relocation type.  This problem is compounded by
>the
>> fact that the arithmetic construct that was previously used to
>> synthesise an @pcl like behaviour, does not work on recent versions
>of
>> binutils.
>>
>> In the commit 20554a78a code was added that selects between the new
>> style @pcl relocations, and the old style arithmetic construct,
>> however, this selection is done based on whether the uClibc user has
>> configured with native threads or not.
>
>Right - the idea at the time was @pcl was added to tools for TLS/NPTL
>support and
>thus uClibc NPTL loosely implied that your tools will have that support
>- but this
>was not the ideal choice I agree.
>
>> Of course, a uClibc user could choose to use a modern version of
>> binutils AND configure without native thread support, in which case
>> uClibc will not compile.
>>
>> I have two proposed solutions.  In patch 1/2 I simply drop support
>for
>> the older versions of binutils in favour of the new @pcl relocation
>> type.  This feels the cleanest solution, but I don't know how
>strongly
>> the uClibc(-ng) community feels about maintaining compatibility for
>> older versions of the tools.
>
>So this was reported recently by Waldemar's build service too.
>
>http://mailman.uclibc-ng.org/pipermail/devel/2016-July/001072.html
>
>And we have an exact same patch floated internally which Vlad sent out
>earlier
>this week
>
>> Given that I anticipated push back against the first patch I took a
>> look at how I might maintain compatibility.
>
>Not really - while indeed 1/2 opens up a non-compatibilty window, there
>is less
>likelihood people will mix-n-match different baselines of uclibc and
>gcc/gas. So
>Vlad's patch does exactly that.
>
>> It turns out to be pretty
>> easy, and that is patch 2/2.  In this patch I drew inspiration from
>> similar examples in the Rules.mak file to check if the toolchain
>> supports @pcl relocations or not.  With this done we have a new
>define
>> based on the specific toolchain feature being supported or not, which
>> can then be used to conditionalise the code.
>
>Indeed your 2/2 seems to be the most "past-proof" code change. So I
>would think it
>is indeed better and is something I should have done in the first
>place.
>
>@Alexey, @Vlad what say you ?

uClibc traditionally supports the current stable release of binutils, which would make it patch #1 I think.

Thanks,
>
>-Vineet
>
>> Would you consider merging one of these patches?
>>
>> Thanks,
>> Andrew

_______________________________________________
uClibc mailing list
[hidden email]
http://lists.busybox.net/mailman/listinfo/uclibc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 2/2] ARC: Add new compiler define to indicate @pcl relocation support

Waldemar Brodkorb
In reply to this post by Andrew Burgess
Hi Andrew,
Andrew Burgess wrote,

> Some old versions of binutils did not support @pcl relocations.  This
> commit adds a new flag to the uClibc configuration system that detects
> if the toolchain supports @pcl relocations or not.
>
> If this relocation is supported then the define ARC_HAS_AT_PCL_RELOC
> will be passed to the compiler, which is then used in the arc ldso to
> choose between generating old or new style code.
>
> This commit addresses and issue that was worked around in commit
> 181d410ad00cddd1d6c9f4835e129136b74c5187 (ARC: Conditionalise certain
> relocations as provided by TLS tools only).  In this commit the choice
> between old or new style relocations was made based on whether uClibc
> was configured with native threads or not.  The problem is that a user
> of a new toolchain might choose to configure without native threads.
> ---
>  Rules.mak                  | 2 ++
>  ldso/ldso/arc/dl-startup.h | 2 +-
>  ldso/ldso/arc/dl-sysdep.h  | 4 ++++
>  3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Rules.mak b/Rules.mak
> index 3c80016..04ff02f 100644
> --- a/Rules.mak
> +++ b/Rules.mak
> @@ -507,9 +507,11 @@ ifeq ($(TARGET_ARCH),c6x)
>  endif
>  
>  ifeq ($(TARGET_ARCH),arc)
> + ARC_HAS_AT_PCL_RELOC:=$(shell echo -e "\t.text\n\tadd r0,pcl,_symbol@pcl" | $(CC) -c -x assembler -o /dev/null -  2> /dev/null && echo -n y || echo -n n)
>   CPU_CFLAGS-y += -mlock -mswape
>   CPU_CFLAGS-$(CONFIG_ARC_CPU_700) += -mA7
>   CPU_CFLAGS-$(CONFIG_ARC_CPU_HS) += -mcpu=archs
> + CPU_CFLAGS-$(ARC_HAS_AT_PCL_RELOC) += -DARC_HAS_AT_PCL_RELOC
>   CPU_LDFLAGS-y += $(CPU_CFLAGS) -marclinux
>  endif

I tried the patch, but get this with arc 2016.03 (ARC700, LE):

/home/wbx/embedded-test/openadk/toolchain_nsim-arcv1_uclibc-ng_arc700/usr/bin/arc-openadk-linux-uclibc-gcc
-c libc/sysdeps/linux/common/pause.c -o
libc/sysdeps/linux/common/pause.os -Wall -Wstrict-prototypes
-Wstrict-aliasing -funsigned-char -fno-builtin -fno-asm
-fmerge-all-constants -std=gnu99 -mlock -mswape -mA7
-fno-stack-protector -nostdinc -I./include -I./include -include
libc-symbols.h -I./libc/sysdeps/linux/arc -I./libc/sysdeps/linux
-I./ldso/ldso/arc -I./ldso/include -I. -Os -fstrict-aliasing -fwrapv
-fno-ident -mcpu=arc700 -Os -pipe -fomit-frame-pointer
-fno-unwind-tables -fno-asynchronous-unwind-tables
-D__USE_STDIO_FUTEXES__ -DHAVE_FORCED_UNWIND -D_LIBC_REENTRANT
-I./libpthread/nptl -I./libpthread/nptl
-I./libpthread/nptl/sysdeps/unix/sysv/linux/arc
-I./libpthread/nptl/sysdeps/arc -I./libpthread/nptl/sysdeps/arc
-I./libpthread/nptl/sysdeps/unix/sysv/linux
-I./libpthread/nptl/sysdeps/unix/sysv/linux
-I./libpthread/nptl/sysdeps/pthread
-I./libpthread/nptl/sysdeps/pthread/bits
-I./libpthread/nptl/sysdeps/generic -I./libc/sysdeps/linux/common
-isystem
/home/wbx/embedded-test/openadk/toolchain_nsim-arcv1_uclibc-ng_arc700/usr/lib/gcc/arc-openadk-linux-uclibc/4.8.5/include-fixed
-isystem
/home/wbx/embedded-test/openadk/toolchain_nsim-arcv1_uclibc-ng_arc700/usr/lib/gcc/arc-openadk-linux-uclibc/4.8.5/include
-I/home/wbx/embedded-test/openadk/target_nsim-arcv1_uclibc-ng_arc700/usr/include/
-DNDEBUG -DIN_LIB=libc -fPIC -fexceptions
-fasynchronous-unwind-tables -MT libc/sysdeps/linux/common/pause.os
-MD -MP -MF libc/sysdeps/linux/common/.pause.os.dep
{standard input}: Assembler messages:
{standard input}:15: Error: invalid operands (.bss and .text
sections) for `-'
{standard input}:15: Error: invalid operands (.text and *ABS*
sections) for `&'
{standard input}:17: Error: invalid operands (.text.exit and .text
sections) for `-'
{standard input}:17: Error: invalid operands (.text and *ABS*
sections) for `&'
Makerules:385: recipe for target 'ldso/ldso/ldso.oS' failed
make[6]: *** [ldso/ldso/ldso.oS] Error 1

It seems ARC_HAS_AT_PCL_RELOC isn't set. I tracked it down, it seems
a combination of my shell and echo -e usage. I know there is another
echo -e usage in Rules.mak and I will fix it later. Can you please
resend the patch with printf instead of echo -e, if the Synopsis ARC
developers agree on this patch?
I added the trailing "\n" to avoid a unnecessary warning when trying
just the command in a shell, sth like that:

diff --git a/Rules.mak b/Rules.mak
index 04ff02f..8b962b4 100644
--- a/Rules.mak
+++ b/Rules.mak
@@ -507,7 +507,7 @@ ifeq ($(TARGET_ARCH),c6x)
 endif
 
 ifeq ($(TARGET_ARCH),arc)
-       ARC_HAS_AT_PCL_RELOC:=$(shell echo -e "\t.text\n\tadd
        r0,pcl,_symbol@pcl" | $(CC) -c -x assembler -o /dev/null -
2> /dev/null && echo -n y || echo -n n)
+       ARC_HAS_AT_PCL_RELOC:=$(shell printf "\t.text\n\tadd
r0,pcl,_symbol@pcl\n" | $(CC) -c -x assembler -o /dev/null - 2>
/dev/null && echo -n y || echo -n n)
        CPU_CFLAGS-y += -mlock -mswape
        CPU_CFLAGS-$(CONFIG_ARC_CPU_700) += -mA7
        CPU_CFLAGS-$(CONFIG_ARC_CPU_HS) += -mcpu=archs

Otherwise looks good to me and fixes my problem with a nothread
build :)

thanks
 Waldemar
_______________________________________________
uClibc mailing list
[hidden email]
http://lists.busybox.net/mailman/listinfo/uclibc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 0/2] ARC Moving to @pcl relocations

Vineet Gupta-5
In reply to this post by Bernhard Reutner-Fischer
On 07/28/2016 03:04 PM, Bernhard Reutner-Fischer wrote:
>> Indeed your 2/2 seems to be the most "past-proof" code change. So I
>> >would think it
>> >is indeed better and is something I should have done in the first
>> >place.
>> >
>> >@Alexey, @Vlad what say you ?
> uClibc traditionally supports the current stable release of binutils, which would make it patch #1 I think.
>

But 2/2 works for both and makes actual binutils version moot. FWIW, ARC tools
don't as of last release didn't use the upstream/stable binutils, but we are
pretty close to that now though.

Thx,
-Vineet
_______________________________________________
uClibc mailing list
[hidden email]
http://lists.busybox.net/mailman/listinfo/uclibc
Loading...