Bug#798955: [PATCH] libstdc++: don't use #include_next in c_global headers
Hi, On Mon, Apr 20, 2020 at 10:12:37AM +0100, Jonathan Wakely wrote: > > Now you are probably going to say that "-isystem /usr/include" is a bad > > idea and that you shouldn't do that. > > Right. > > > I'm inclined to agree. This isn't a > > problem just yet. Debian wants to move /usr/include/stdlib.h to > > /usr/include//stdlib.h. After that move, the problematic flag > > becomes "-isystem /usr/include/". Unfortunately, around 30 > > Debian packages[1] do pass exactly that flag. Regardless whether doing > > so is a bad idea, I guess we will have to support that. > > Or Debian should fix what they're going to break. This is not quite precise. The offending -isystem /usr/include/ flag is already being passed. According to what you write later, doing so is broken today. It just happens to work by accident. So all we do is making the present breakage visible. > > I am proposing to replace those two #include_next with plain #include. > > That'll solve the problem described above, but it is not entirely > > obvious that doing so doesn't break something else. > > > > After switching those #include_next to #include, > > libstdc++-v3/include/c_global/cstdlib will continue to temporarily > > will #include . Now, it'll search all include directories. It > > may find libstdc++-v3/include/c_comaptibility/stdlib.h or the libc's > > version. We cannot tell which. If it finds the one from libstdc++-v3, > > the header will notice the _GLIBCXX_INCLUDE_NEXT_C_HEADERS macro and > > immediately #include_next skipping the rest of the header. > > That in turn will find the libc version. So in both cases, it ends up > > using the right one. Precisely what we wanted. > > As Marc said, this doesn't work. That is not very precise either. Marc said that it won't fix all cases. In practice, it would make those work that don't #include but use #include instead. Marc also indicated that using include_next for a header of a different name is wrong. So this is a bug in libstdc++ regardless of whether it breaks or unbreaks other pieces of software. > If a program tries to include it needs to get the libstdc++ > version, otherwise only the libc versions of certain functions are > defined. That means the additional C++ overloads such as ::abs(long) > and ::abs(long long) won't be defined. That is the reason why > libstdc++ provides its own . > > And if you do -isystem /usr/include (or any other option that causes > libstdc++'s to be skipped) that doesn't work. Only > ::abs(int) gets defined. > > So -isystem /usr/include breaks code, with or without your patch. It is very difficult to disagree with -isystem /usr/include or -isystem /usr/include/ being broken and unsupported. Having you state it that clearly does help with communicating to other upstreams. For this reason, I've looked into the remaining cases. It turns out that there aren't that many left. In particular chromium, opencv and vtk got fixed in the mean time. Basically all remaining failures could be attributed to qmake, which passes all directories below /usr/include (including /usr/include and /usr/include/ if a .pc file mentions them) using -isystem. I've sent a patch https://bugs.debian.org/958479 to make qmake stop doing that. I therefore agree with you that the patch I sent for libstdc++ is not necessary to make packages build on Debian. Removing the offending -isystem flags from the respective builds is a manageable option and has already happened to a large extend. We can conclude that the motivation for my patch is not a good one, because it embraces broken behaviour. However, the use of include_next remains a bug, because the name of the including and the name of the included header differ, and it should be fixed on that ground. Helmut
Bug#91815: Patch to add memusage and memusagestat
Hi Aurelien and Stephen, On Wed, Apr 22, 2020 at 12:04:32AM +0200, Aurelien Jarno wrote: > > The attached patch adds memusage and memusagestat to the libc-bin package. > > This does mean that the latter becomes dependent on libgd3, so it might be > > better to add a new memusage package; I can take care of that if the > > maintainers think it’s better. > > I am not sure we want to add a new dependency for libc-bin, I am sure > people running embedded systems won't appreciate. Any reason for not > shipping it in libc-dev-bin instead? For me that looks more like a tool > to be used for "development". At least the memusagestat is similar to > the mtrace one that is in libc-dev-bin. > > We also have to make sure that this new build-dependency doesn't break > bootstrapping. I have added Helmut in Cc so that he can have a look. Thank you for notifying me. Indeed, the patch doesn't work for bootstrapping as is. A stage2 build of glibc would start requiring libgd-dev -> libgd3 -> libc6, but the stage1 libc6 will not be sufficient to build src:libgd2. It must be possible to build stage2 without that dependency. (Note that this is going to become more confusing as there is ongoing work on removing stage1 and maybe renaming stage2, but let's stick to the current names for now.) >From a bootstrapping pov, the libgd-dev dependency is similar to the libselinux-dev dependency, but I notice just now that it is not correctly annotated with in Build-Depends! >From a build profile pov, it is very undesirable to have package contents change with profiles. Doing so makes it impossible to validate them using reproducible builds. However, since we need libc6-dev from stage2 and it depends on libc-dev-bin we cannot skip generating libc-dev-bin in stage2. I wonder whether it would make sense to add another binary package for development tools that are not normally needed for building software. Then we could skip generating that package. mtrace would be a good candidate to join that package indeed. Failing that, we'll have to cope with changing package contents for stage2 and disable the new dependency for stage2 (or some other profile). Helmut
Processed: qmake passes unsupported -isystem to gcc
Processing control commands: > affects -1 + src:deepin-music Bug #958479 [qt5-qmake] qmake passes unsupported -isystem to gcc Added indication that 958479 affects src:deepin-music > block 798955 by -1 Bug #798955 [src:glibc] Moving glibc headers from /usr/include to /usr/include/$(DEB_HOST_MULTIARCH) 798955 was blocked by: 911860 911456 875921 909967 911556 909965 909964 911359 909969 911491 909966 910055 911358 912422 911393 910322 911455 910097 911489 912605 910235 911394 911683 857535 910299 911574 910127 857708 798955 was not blocking any bugs. Added blocking bug(s) of 798955: 958479 -- 798955: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=798955 958479: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=958479 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Bug#956418: src:glibc: Please provide optimized builds for ARMv8.1
* Noah Meyerhans: > On Sun, Apr 12, 2020 at 12:18:35PM +0200, Aurelien Jarno wrote: >> > Significant performance impact has also been observed in less contrived >> > cases (MariaDB and Postgres), but I don't have a repro to share. >> >> But indeed what counts is number on real workloads. It would be nice to >> get numbers when those software are run against a rebuilt glibc. As >> those software are using a lot of atomics directly, it would be also >> interesting to have numbers with those software also rebuilt to use >> those new instructions. > > Agreed. I don't have specific examples of real world impact at the > moment. AIUI, the most significant impact comes in the usage of atomics > in pthread_mutex_lock(). When there are multiple threads contending for > a lock, one thread will (approximately) always obtain the lock, while > the others will starve. With atomics support in place, the probability > of obtaining the lock is roughly evenly distributed among all the > threads. So any workload in which multiple threads may contend for a > lock should be a candidate to demonstrate this problem in the real > world. Does this behavior affect just one implementation with LSE, or also implementations without LSE? If the latter, we might need a different mutex implementation for AArch64. 8-(
Bug#956418: src:glibc: Please provide optimized builds for ARMv8.1
On Wed, Apr 22, 2020 at 05:48:27PM +0100, Steve McIntyre wrote: > Hi folks! Hiya, > > I'm adding a CC to Steve Capper, a colleague in Arm who's our expert > here for this kind of question. He's also a DM in Debian... :-) Now I feel guilty about not doing enough Debian :-). > > On Tue, Apr 21, 2020 at 06:37:07PM -0400, Noah Meyerhans wrote: > >On Sun, Apr 12, 2020 at 12:18:35PM +0200, Aurelien Jarno wrote: > > > >> It would also be nice to have numbers to see the impact on non-ARMv8.1 > >> CPU on real workloads. As pointed out by Florian, and if the impact is > >> negligible, it might be a good idea to enable -moutline-atomics > >> globally at the GCC level so that all software can benefit from it, and > >> instead of only glibc. That could be either upstream or only in Debian, > >> that's probably a separate discussion. Otherwise we will likely end up > >> using this non-default GCC option on all packages that runs faster with > >> it. > > > >Agreed. > > I think the -moutline-atomics is probably good to enable by default > once we've got it (gcc 10). that's the suggestion I've heard from gcc > folks in Arm. > > >> Also note that the mechanism allowing a safe upgrade *does* incur a > >> runtime overhead as every binary now has to test for the presence of > >> /etc/ld.so.nohwcap to detect a possible upgrade of the glibc in > >> progress. That's why we have disabled it on architecture not providing > >> an optimized library [1]. > > Oh, ick. :-/ > > >Thanks for the pointer, it's interesting to see data on that. This also > >suggests that it might be worthwhile to investigate a better mechanism > >for identifying the availability of hardware features. > > > >> > I've tested both options and found them to be acceptable on v8.1a > >> > (Neoverse > >> > N1) and v8a (Cortex A72) CPUs. I can provide bulk test run data of the > >> > various different configuration permutations if you'd like to see > >> > additional > >> > data. That's good to hear! > >> > >> As said above I think we would need more numbers on real workload to > >> take a decision. Don't get me wrong I do not oppose on improving atomics > >> on ARMv8.1, but I would like that we chose the best option. Also if we > >> go with the -moutline-atomics option, I believe it rather has to be a > >> ARM porters decision than a glibc maintainers decision (hence the Cc:). > > > >I'll see what I can come up with. > > > >Do the arm porters have any opinions on this matter? > > It's a good question, and thanks for asking! I definitely think it's > worth doing -moutline-atomics, and I'm hoping Steve can share some > performance numbers to help convince. :-) > We ran -moutline-atomics on a mixture of development hardware running, IIRC some DPDK lock tests that employed C11-style atomics. As expected there was a performance penalty, but it was order of magnitude of 1%. The perf boost from moving to LSE was a lot larger (and we noticed the variance dropping a lot with LSE too). FWIW, I'd recommend the -moutline-atomics for the general case. (I used to be a fan of the multi-lib approach; but the way the runtime selection is implemented in gcc with a direct branch changed my mind :-) ). Cheers, -- Steve
Bug#956418: src:glibc: Please provide optimized builds for ARMv8.1
On Wed, Apr 22, 2020 at 01:08:46PM -0400, Noah Meyerhans wrote: >On Wed, Apr 22, 2020 at 05:48:27PM +0100, Steve McIntyre wrote: >> I think the -moutline-atomics is probably good to enable by default >> once we've got it (gcc 10). that's the suggestion I've heard from gcc >> folks in Arm. > >JFTR, it's been backported to gcc 9 and is available in Debian's gcc-9 >as of 9.3.0-9. See >https://salsa.debian.org/toolchain-team/gcc/-/blob/gcc-9-debian/debian/patches/git-updates.diff Ah, cool. I knew it *was* being backported, but I wasn't aware it was already with us. Woot! -- Steve McIntyre, Cambridge, UK.st...@einval.com Getting a SCSI chain working is perfectly simple if you remember that there must be exactly three terminations: one on one end of the cable, one on the far end, and the goat, terminated over the SCSI chain with a silver-handled knife whilst burning *black* candles. --- Anthony DeBoer
Bug#956418: src:glibc: Please provide optimized builds for ARMv8.1
On Wed, Apr 22, 2020 at 05:48:27PM +0100, Steve McIntyre wrote: > I think the -moutline-atomics is probably good to enable by default > once we've got it (gcc 10). that's the suggestion I've heard from gcc > folks in Arm. JFTR, it's been backported to gcc 9 and is available in Debian's gcc-9 as of 9.3.0-9. See https://salsa.debian.org/toolchain-team/gcc/-/blob/gcc-9-debian/debian/patches/git-updates.diff noah
Bug#956418: src:glibc: Please provide optimized builds for ARMv8.1
Hi folks! I'm adding a CC to Steve Capper, a colleague in Arm who's our expert here for this kind of question. He's also a DM in Debian... :-) On Tue, Apr 21, 2020 at 06:37:07PM -0400, Noah Meyerhans wrote: >On Sun, Apr 12, 2020 at 12:18:35PM +0200, Aurelien Jarno wrote: > >> It would also be nice to have numbers to see the impact on non-ARMv8.1 >> CPU on real workloads. As pointed out by Florian, and if the impact is >> negligible, it might be a good idea to enable -moutline-atomics >> globally at the GCC level so that all software can benefit from it, and >> instead of only glibc. That could be either upstream or only in Debian, >> that's probably a separate discussion. Otherwise we will likely end up >> using this non-default GCC option on all packages that runs faster with >> it. > >Agreed. I think the -moutline-atomics is probably good to enable by default once we've got it (gcc 10). that's the suggestion I've heard from gcc folks in Arm. >> Also note that the mechanism allowing a safe upgrade *does* incur a >> runtime overhead as every binary now has to test for the presence of >> /etc/ld.so.nohwcap to detect a possible upgrade of the glibc in >> progress. That's why we have disabled it on architecture not providing >> an optimized library [1]. Oh, ick. :-/ >Thanks for the pointer, it's interesting to see data on that. This also >suggests that it might be worthwhile to investigate a better mechanism >for identifying the availability of hardware features. > >> > I've tested both options and found them to be acceptable on v8.1a (Neoverse >> > N1) and v8a (Cortex A72) CPUs. I can provide bulk test run data of the >> > various different configuration permutations if you'd like to see >> > additional >> > data. >> >> As said above I think we would need more numbers on real workload to >> take a decision. Don't get me wrong I do not oppose on improving atomics >> on ARMv8.1, but I would like that we chose the best option. Also if we >> go with the -moutline-atomics option, I believe it rather has to be a >> ARM porters decision than a glibc maintainers decision (hence the Cc:). > >I'll see what I can come up with. > >Do the arm porters have any opinions on this matter? It's a good question, and thanks for asking! I definitely think it's worth doing -moutline-atomics, and I'm hoping Steve can share some performance numbers to help convince. :-) -- Steve McIntyre, Cambridge, UK.st...@einval.com Who needs computer imagery when you've got Brian Blessed?
Bug#91815: Patch to add memusage and memusagestat
Hi, On Wed, 22 Apr 2020 00:04:32 +0200, Aurelien Jarno wrote: > On 2020-04-21 20:58, Stephen Kitt wrote: > Thanks for trying to fix one of the oldest glibc bugs ;-) You’re welcome! > > The attached patch adds memusage and memusagestat to the libc-bin package. > > This does mean that the latter becomes dependent on libgd3, so it might be > > better to add a new memusage package; I can take care of that if the > > maintainers think it’s better. > > I am not sure we want to add a new dependency for libc-bin, I am sure > people running embedded systems won't appreciate. Any reason for not > shipping it in libc-dev-bin instead? For me that looks more like a tool > to be used for "development". At least the memusagestat is similar to > the mtrace one that is in libc-dev-bin. Indeed, especially since libgd3 is a rather heavy-weight dependency. I’m attaching a patch which adds the tools to libc-dev-bin instead. > We also have to make sure that this new build-dependency doesn't break > bootstrapping. I have added Helmut in Cc so that he can have a look. Isn’t non-cross bootstrapping handled by the stage1 build profile? If this causes issues with cross bootstrapping then shipping a separate memusage package ( ) should take care of things. Regards, Stephen From bd3f09edb6296b8a8e2987aefd3169d48903625f Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Tue, 21 Apr 2020 20:55:53 +0200 Subject: [PATCH] Build and package memusage* This builds memusage and memusagestat in the libc pass, and ships them in libc-dev-bin. This involves adding a build-dependency on libgd-dev (outside stage1) and libc-dev-bin acquires a runtime dependency on the corresponding library package. Closes: #91815 Signed-off-by: Stephen Kitt --- debian/control.in/main | 3 ++- debian/debhelper.in/libc-dev-bin.install | 2 ++ debian/debhelper.in/libc-dev-bin.lintian-overrides | 2 ++ debian/rules.d/build.mk| 6 +- 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/debian/control.in/main b/debian/control.in/main index 659267bd..3537b751 100644 --- a/debian/control.in/main +++ b/debian/control.in/main @@ -12,7 +12,8 @@ Build-Depends: gettext, dpkg (>= 1.18.7), dpkg-dev (>= 1.17.14), xz-utils, file, g++-9, g++-9-multilib [amd64 i386 kfreebsd-amd64 mips mipsel mipsn32 mipsn32el mips64 mips64el mipsr6 mipsr6el mipsn32r6 mipsn32r6el mips64r6 mips64r6el powerpc ppc64 s390x sparc sparc64 x32] , python3:native, libidn2-0 (>= 2.0.5~) , - libc-bin (>= @GLIBC_VERSION@) + libc-bin (>= @GLIBC_VERSION@) , + libgd-dev Build-Depends-Indep: perl, po-debconf (>= 1.0) Maintainer: GNU Libc Maintainers Uploaders: Clint Adams , Aurelien Jarno , Adam Conrad , Samuel Thibault diff --git a/debian/debhelper.in/libc-dev-bin.install b/debian/debhelper.in/libc-dev-bin.install index 0d760a7e..8ced4378 100644 --- a/debian/debhelper.in/libc-dev-bin.install +++ b/debian/debhelper.in/libc-dev-bin.install @@ -1,4 +1,6 @@ debian/tmp-libc/usr/bin/gencat usr/bin +debian/tmp-libc/usr/bin/memusage usr/bin +debian/tmp-libc/usr/bin/memusagestat usr/bin debian/tmp-libc/usr/bin/mtrace usr/bin debian/tmp-libc/usr/bin/rpcgen usr/bin debian/tmp-libc/usr/bin/sotruss usr/bin diff --git a/debian/debhelper.in/libc-dev-bin.lintian-overrides b/debian/debhelper.in/libc-dev-bin.lintian-overrides index eedd7cbd..1031449a 100644 --- a/debian/debhelper.in/libc-dev-bin.lintian-overrides +++ b/debian/debhelper.in/libc-dev-bin.lintian-overrides @@ -1,3 +1,5 @@ # these manpages are provided by the manpages package +libc-dev-bin: binary-without-manpage usr/bin/memusage +libc-dev-bin: binary-without-manpage usr/bin/memusagestat libc-dev-bin: binary-without-manpage usr/bin/mtrace libc-dev-bin: binary-without-manpage usr/bin/sprof diff --git a/debian/rules.d/build.mk b/debian/rules.d/build.mk index 0d03116a..3f664316 100644 --- a/debian/rules.d/build.mk +++ b/debian/rules.d/build.mk @@ -37,7 +37,11 @@ $(stamp)configure_%: $(stamp)config_sub_guess $(stamp)patch $(KERNEL_HEADER_DIR) echo "BASH := /bin/bash" >> $(DEB_BUILDDIR)/configparms echo "KSH := /bin/bash" >> $(DEB_BUILDDIR)/configparms echo "SHELL := /bin/bash" >> $(DEB_BUILDDIR)/configparms - echo "LIBGD = no" >> $(DEB_BUILDDIR)/configparms + if [ "$(curpass)" = "libc" ]; then \ + echo "LIBGD = yes" >> $(DEB_BUILDDIR)/configparms; \ + else \ + echo "LIBGD = no" >> $(DEB_BUILDDIR)/configparms; \ + fi echo "bindir = $(bindir)" >> $(DEB_BUILDDIR)/configparms echo "datadir = $(datadir)" >> $(DEB_BUILDDIR)/configparms echo "complocaledir = $(complocaledir)" >> $(DEB_BUILDDIR)/configparms -- 2.20.1 pgpEALW2r4ByK.pgp Description: OpenPGP digital signature
