[v2,3/3] Consolidate posix_fadvise implementations

Message ID 3faeab56-ea0f-1fb1-68cd-f260162b18ab@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Netto Oct. 12, 2016, 4:04 p.m. UTC
  On 12/10/2016 12:39, Joseph Myers wrote:
> On Wed, 12 Oct 2016, Adhemerval Zanella wrote:
> 
>> For some reason I am not seeing this issue with my mips64n64 toolchain
>> (gcc 5.3.1, binutils 2.26.0.20160331).
> 
> I was using GCC 5.4.1 20160810 and binutils 2.27.51.20160810.  I've now 
> updated to today's GCC 5 branch and binutils master, and still see the 
> same issue, in a clean build from scratch.

I updated the binutils in my mips64 toolchain to master (2.27.51.20161012)
and now I am see this behaviour.

> 
> The error is complaining about libc.so when linking sotruss-lib.so.  
> libc.so has (readelf --dyn-syms)
> 
>    262: 000000000010b950    28 FUNC    GLOBAL DEFAULT   12 posix_fadvise64@GLIBC_2.2
>    417: 000000000010b950    28 FUNC    WEAK   DEFAULT   12 posix_fadvise64@@GLIBC_2.2
>   1505: 000000000010b950    28 FUNC    GLOBAL DEFAULT   12 posix_fadvise64@@GLIBC_2.3.3
> 
> that is, two separate definitions at version GLIBC_2.2.  It looks to me 
> like sysdeps/unix/sysv/linux/posix_fadvise64.c would create a 
> posix_fadvise64 weak_alias, while 
> sysdeps/unix/sysv/linux/mips/mips64/n64/posix_fadvise64.c then adds 
> compat_symbol / versioned_symbol calls.  If a symbol is being created with 
> explicit versioning, direct weak_alias / strong_alias calls for it should 
> be disabled.

Yes, it seems the case.  The patch below disable the weak_alias for mips64
and I could complete the build without issues with check-abi clean.

By the way, do you know exactly why mips64n64 requires this symbol versioning?
I noted MIPS64 sets __OFF_T_MATCHES_OFF64_T and it is ILP64 (so size_t is
8 bytes), so 

--
  

Comments

Joseph Myers Oct. 12, 2016, 4:10 p.m. UTC | #1
On Wed, 12 Oct 2016, Adhemerval Zanella wrote:

> By the way, do you know exactly why mips64n64 requires this symbol versioning?

See what I said in 
<https://sourceware.org/ml/libc-ports/2012-05/msg00006.html>.  Because 
there were releases with both symbol versions, we need to keep the exports 
at both versions even though they have the same semantics.

Please commit the patch.
  
Adhemerval Zanella Netto Oct. 12, 2016, 4:14 p.m. UTC | #2
On 12/10/2016 13:10, Joseph Myers wrote:
> On Wed, 12 Oct 2016, Adhemerval Zanella wrote:
> 
>> By the way, do you know exactly why mips64n64 requires this symbol versioning?
> 
> See what I said in 
> <https://sourceware.org/ml/libc-ports/2012-05/msg00006.html>.  Because 
> there were releases with both symbol versions, we need to keep the exports 
> at both versions even though they have the same semantics.
> 
> Please commit the patch.
> 

Thanks, I will add a comment on mips64 posix_fadvise64 implementation
explaining why it is required.
  

Patch

diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/posix_fadvise64.c b/sysdeps/unix/sysv/linux/mips/mips64/n64/posix_fadvise64.c
index d7aab25..0af3c38 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/posix_fadvise64.c
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/posix_fadvise64.c
@@ -15,6 +15,8 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#undef weak_alias
+#define weak_alias(a, b)
 #undef strong_alias
 #define strong_alias(a, b)