This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]
Re: [PATCH] change GLIBC PPC64/ELF2 ABI default to 2.17

From: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
To: Roland McGrath <roland at hack dot frob dot com>, "Joseph S. Myers" <joseph at codesourcery dot com>
Cc: munroesj at us dot ibm dot com, Adam Conrad <adconrad at 0c3 dot net>, Brooks Moses <brooks dot moses at dpdx dot net>, libc-alpha at sourceware dot org, carlos at redhat dot com
Date: Sun, 02 Feb 2014 16:16:25 -0200
Subject: Re: [PATCH] change GLIBC PPC64/ELF2 ABI default to 2.17
Authentication-results: sourceware.org; auth=none
References: <20140131201607 dot GG99202 at jinx> <1391202081 dot 1683 dot 17 dot camel at spokane1 dot rchland dot ibm dot com> <20140131213241 dot GP15976 at 0c3 dot net> <20140131215927 dot 769937441B at topped-with-meat dot com> <1391209204 dot 1683 dot 95 dot camel at spokane1 dot rchland dot ibm dot com> <Pine dot LNX dot 4 dot 64 dot 1401312306020 dot 30733 at digraph dot polyomino dot org dot uk> <20140131233404 dot 9179374419 at topped-with-meat dot com>
On 31-01-2014 21:34, Roland McGrath wrote:
>> <https://sourceware.org/ml/libc-alpha/2013-08/msg00096.html> was I think 
>> Alan Modra's original proposal for ABI baselines.  There were concerns 
>> about the piles of Implies files that needs, but I think it's better to 
>> get the ABI baselines checked in, and possibly clean up the directory 
>> structures later, than to leave them out and risk ABI breakage that will 
>> be hard to detect and fix.
> I reluctantly tend to agree.
>
> Certainly it should not be considered a viable option to ship a release
> that (newly) claims to support some ABI but does not have complete abilist
> files for that ABI.
>
> I think it's really not a lot of work to change the way we find the abilist
> files.  I'd previously said we should really figure out our plan to revamp
> shlib-versions and meld the abilist cleanup with that.  Doing all that is
> clearly too much work and too much churn for this point in the release
> freeze.  Just doing something quick to support finding abilist files
> differently is probably pretty easy, so I'm inclined to see if we can get
> it done right quick.  But if not, then an Implies horror for powerpc64le is
> certainly better than having no check-abi safety at all.  If it does go
> that way, I'd want a commitment from someone or other to clean things up
> promptly so we don't go another release with increased Implies horrors.
> That was a constraint I tried to impose when we took the dismal
> Implies-{before,after} hacks for x32, but we never followed up.
> Fortunately I think all the shlib-versions, abilist, and related revamps
> are far easier to figure out and decide upon than the sysdeps selection
> revamp.
>
> Here is a patch (also on branch roland/abilist) that is wholly untested and
> not necessarily well thought-through.  But I think it might just cover it.
>
>
> Thanks,
> Roland
>
>
> 2014-01-31  Roland McGrath  <roland@hack.frob.com>
>
> 	* sysdeps/powerpc/Makefile [$(config-machine) ends with 'le']
> 	(abilist-pattern): New variable, set to %-le.abilist.
>
> 	* Makerules (abilist-pattern): New variable.
> 	(vpath): Use $(abilist-pattern) in place of %.abilist.
> 	(check-abi-% pattern rule): Likewise.
> 	(check-abi, update-abi): Likewise.
>
> --- a/Makerules
> +++ b/Makerules
> @@ -1175,7 +1175,15 @@ ifeq ($(build-shared),yes)
>  	LC_ALL=C $(OBJDUMP) --dynamic-syms $< > $@T
>  	mv -f $@T $@
>
> -vpath %.abilist $(+sysdep_dirs)
> +# A sysdeps/.../Makefile can set abilist-pattern to something like
> +# %-foo.abilist to look for libc-foo.abilist instead of libc.abilist.
> +# This makes sense if multiple ABIs can be most cleanly supported by a
> +# configuration without using separate sysdeps directories for each.
> +ifndef abilist-pattern
> +abilist-pattern = %.abilist
> +endif
> +
> +vpath $(abilist-pattern) $(+sysdep_dirs)
>
>  # The .PRECIOUS rule prevents the files built by an implicit rule whose
>  # target pattern is %.symlist from being considered "intermediate files"
> @@ -1184,24 +1192,25 @@ vpath %.abilist $(+sysdep_dirs)
>  .PRECIOUS: %.symlist
>  generated += $(extra-libs:=.symlist)
>
> -check-abi-%: $(common-objpfx)config.make %.abilist $(objpfx)%.symlist
> +check-abi-%: $(common-objpfx)config.make $(abilist-pattern) $(objpfx)%.symlist
>  	$(check-abi)
> -check-abi-%: $(common-objpfx)config.make %.abilist $(common-objpfx)%.symlist
> +check-abi-%: $(common-objpfx)config.make $(abilist-pattern) \
> +	     $(common-objpfx)%.symlist
>  	$(check-abi)
>  define check-abi
> -	diff -p -U 0 $(filter %.abilist,$^) $(filter %.symlist,$^)
> +	diff -p -U 0 $(filter $(abilist-pattern),$^) $(filter %.symlist,$^)
>  endef
>
> -update-abi-%: $(objpfx)%.symlist %.abilist
> +update-abi-%: $(objpfx)%.symlist $(abilist-pattern)
>  	$(update-abi)
> -update-abi-%: $(common-objpfx)%.symlist %.abilist
> +update-abi-%: $(common-objpfx)%.symlist $(abilist-pattern)
>  	$(update-abi)
>  define update-abi
>  @if cmp -s $^ 2> /dev/null; \
>   then \
> -      echo '+++ $(filter %.abilist,$^) is unchanged'; \
> +      echo '+++ $(filter $(abilist-pattern),$^) is unchanged'; \
>   else cp -f $^; \
> -      echo '*** Now check $(filter %.abilist,$^) changes for correctness ***'; \
> +      echo '*** Now check $(filter $(abilist-pattern),$^) changes for correctness ***'; \
>   fi
>  endef
>
> --- a/sysdeps/powerpc/Makefile
> +++ b/sysdeps/powerpc/Makefile
> @@ -27,3 +27,7 @@ ifeq ($(subdir),misc)
>  sysdep_headers += sys/platform/ppc.h
>  tests += test-gettimebase
>  endif
> +
> +ifneq (,$(filter %le,$(config-machine)))
> +abilist-pattern = %-le.abilist
> +endif
>

Hi Roland,

Thanks for the patch and I checked it with a LE build. The current issue is the implicit
rules won't matches the generic ABI files at sysdeps/generic and thus powerpc64le
will need to duplicate the files in the powerpc64 abi folder (which I think it is not
really smart).

Below a modification based on patch to create implicit rules if abilist-pattern with
keeping the default rules for *.abifiles. I check with a powerpc64le and a x86_64 build
and looks like expected:

[powerpc64le build]
$ make -C /home/azanella/glibc/glibc-git objdir=`pwd` check-abi 2>&1 | tee _check_abi | grep diff
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libBrokenLocale-le.abilist /home/azanella/glibc/glibc-git-build-power7/locale/libBrokenLocale.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libm-le.abilist /home/azanella/glibc/glibc-git-build-power7/math/libm.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libdl-le.abilist /home/azanella/glibc/glibc-git-build-power7/dlfcn/libdl.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libcrypt-le.abilist /home/azanella/glibc/glibc-git-build-power7/crypt/libcrypt.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libpthread-le.abilist /home/azanella/glibc/glibc-git-build-power7/nptl/libpthread.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/librt-le.abilist /home/azanella/glibc/glibc-git-build-power7/rt/librt.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libthread_db-le.abilist /home/azanella/glibc/glibc-git-build-power7/nptl_db/libthread_db.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libresolv-le.abilist /home/azanella/glibc/glibc-git-build-power7/resolv/libresolv.symlist
diff -p -U 0 ../sysdeps/generic/libnss_dns.abilist /home/azanella/glibc/glibc-git-build-power7/resolv/libnss_dns.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libanl-le.abilist /home/azanella/glibc/glibc-git-build-power7/resolv/libanl.symlist
diff -p -U 0 ../sysdeps/generic/libnss_files.abilist /home/azanella/glibc/glibc-git-build-power7/nss/libnss_files.symlist
diff -p -U 0 ../sysdeps/generic/libnss_db.abilist /home/azanella/glibc/glibc-git-build-power7/nss/libnss_db.symlist
diff -p -U 0 ../sysdeps/generic/libnss_hesiod.abilist /home/azanella/glibc/glibc-git-build-power7/hesiod/libnss_hesiod.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libnsl-le.abilist /home/azanella/glibc/glibc-git-build-power7/nis/libnsl.symlist
diff -p -U 0 ../sysdeps/generic/libnss_nis.abilist /home/azanella/glibc/glibc-git-build-power7/nis/libnss_nis.symlist
diff -p -U 0 ../sysdeps/generic/libnss_nisplus.abilist /home/azanella/glibc/glibc-git-build-power7/nis/libnss_nisplus.symlist
diff -p -U 0 ../sysdeps/generic/libnss_compat.abilist /home/azanella/glibc/glibc-git-build-power7/nis/libnss_compat.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libutil-le.abilist /home/azanella/glibc/glibc-git-build-power7/login/libutil.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libc-le.abilist /home/azanella/glibc/glibc-git-build-power7/libc.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/ld-le.abilist /home/azanella/glibc/glibc-git-build-power7/elf/ld.symlist

[x86_64 build]
$ make -C /home/azanella/glibc/glibc-git objdir=`pwd` check-abi 2>&1 | tee _check_abi | grep diff
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libBrokenLocale.abilist /home/azanella/glibc/glibc-git-build-m64/locale/libBrokenLocale.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libm.abilist /home/azanella/glibc/glibc-git-build-m64/math/libm.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libdl.abilist /home/azanella/glibc/glibc-git-build-m64/dlfcn/libdl.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libcrypt.abilist /home/azanella/glibc/glibc-git-build-m64/crypt/libcrypt.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libpthread.abilist /home/azanella/glibc/glibc-git-build-m64/nptl/libpthread.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/librt.abilist /home/azanella//libc/glibc-git-build-m64/rt/librt.symlist
diff -p -U 0 ../sysdeps/generic/libcidn.abilist /home/azanella/glibc/glibc-git-build-m64/libidn/libcidn.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libthread_db.abilist /home/azanella/glibc/glibc-git-build-m64/nptl_db/libthread_db.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libresolv.abilist /home/azanella/gliPbc/glibc-git-build-m64/resolv/libresolv.symlist
diff -p -U 0 ../sysdeps/generic/libnss_dns.abilist /home/azanella/glibc/glibc-git-build-m64/resolv/libnss_dns.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libanl.abilist /home/azanella/glibc/glibc-git-build-m64/resolv/libanl.symlist
diff -p -U 0 ../sysdeps/generic/libnss_files.abilist /home/azanella/glibc/glibc-git-build-m64/nss/libnss_files.symlist
diff -p -U 0 ../sysdeps/generic/libnss_db.abilist /home/azanella/glibc/glibc-git-build-m64/nss/libnss_db.symlist
diff -p -U 0 ../sysdeps/generic/libnss_hesiod.abilist /home/azanella/glibc/glibc-git-build-m64/hesiod/libnss_hesiod.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libnsl.abilist /home/azanella/glibc/glibc-git-build-m64/nis/libnsl.symlist
diff -p -U 0 ../sysdeps/generic/libnss_nis.abilist /home/azanella/glibc/glibc-git-build-m64/nis/libnss_nis.symlist
diff -p -U 0 ../sysdeps/generic/libnss_nisplus.abilist /home/azanella/glibc/glibc-git-build-m64/nis/libnss_nisplus.symlist
diff -p -U 0 ../sysdeps/generic/libnss_compat.abilist /home/azanella/glibc/glibc-git-build-m64/nis/libnss_compat.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libutil.abilist /home/azanella/glibc/glibc-git-build-m64/login/libutil.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/libc.abilist /home/azanella/glibc/glibc-git-build-m64/libc.symlist
diff -p -U 0 ../sysdeps/unix/sysv/linux/x86_64/64/nptl/ld.abilist /home/azanella/glibc/glibc-git-build-m64/elf/ld.symlist

What do you think?

---

diff --git a/Makerules b/Makerules
index b7e556f..b46b09b 100644
--- a/Makerules
+++ b/Makerules
@@ -1175,6 +1175,14 @@ ifeq ($(build-shared),yes)
 	LC_ALL=C $(OBJDUMP) --dynamic-syms $< > $@T
 	mv -f $@T $@
 
+# A sysdeps/.../Makefile can set abilist-pattern to something like
+# %-foo.abilist to look for libc-foo.abilist before of libc.abilist.
+# This makes sense if multiple ABIs can be most cleanly supported by a
+# configuration without using separate sysdeps directories for each.
+ifdef abilist-pattern
+vpath $(abilist-pattern) $(+sysdep_dirs)
+endif
+
 vpath %.abilist $(+sysdep_dirs)
 
 # The .PRECIOUS rule prevents the files built by an implicit rule whose
@@ -1184,18 +1192,42 @@ vpath %.abilist $(+sysdep_dirs)
 .PRECIOUS: %.symlist
 generated += $(extra-libs:=.symlist)
 
+ifdef abilist-pattern
+check-abi-%: $(common-objpfx)config.make $(abilist-pattern) $(objpfx)%.symlist
+	$(check-abi-pattern)
+check-abi-%: $(common-objpfx)config.make $(abilist-pattern) \
+	     $(common-objpfx)%.symlist
+	$(check-abi-pattern)
+endif
 check-abi-%: $(common-objpfx)config.make %.abilist $(objpfx)%.symlist
 	$(check-abi)
 check-abi-%: $(common-objpfx)config.make %.abilist $(common-objpfx)%.symlist
 	$(check-abi)
+define check-abi-pattern
+	diff -p -U 0 $(filter $(abilist-pattern),$^) $(filter %.symlist,$^)
+endef
 define check-abi
 	diff -p -U 0 $(filter %.abilist,$^) $(filter %.symlist,$^)
 endef
 
+ifdef abilist-pattern
+update-abi-%: $(objpfx)%.symlist $(abilist-pattern)
+	$(update-abi-pattern)
+update-abi-%: $(common-objpfx)%.symlist $(abilist-pattern)
+	$(update-abi-pattern)
+endif
 update-abi-%: $(objpfx)%.symlist %.abilist
 	$(update-abi)
 update-abi-%: $(common-objpfx)%.symlist %.abilist
 	$(update-abi)
+define update-abi-pattern
+@if cmp -s $^ 2> /dev/null; \
+ then \
+      echo '+++ $(filter $(abilist-pattern),$^) is unchanged'; \
+ else cp -f $^; \
+      echo '*** Now check $(filter $(abilist-pattern),$^) changes for correctness ***'; \
+ fi
+endef
 define update-abi
 @if cmp -s $^ 2> /dev/null; \
  then \
diff --git a/sysdeps/powerpc/Makefile b/sysdeps/powerpc/Makefile
index f75e625..b11edd7 100644
--- a/sysdeps/powerpc/Makefile
+++ b/sysdeps/powerpc/Makefile
@@ -27,3 +27,7 @@ ifeq ($(subdir),misc)
 sysdep_headers += sys/platform/ppc.h
 tests += test-gettimebase
 endif
+
+ifneq (,$(filter %le,$(config-machine)))
+abilist-pattern = %-le.abilist
+endif
Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]