Message ID | 20190205211544.26547-1-patrickdepinguin@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | toolchain-external: restrict copying of dynamic loader to ld*.so.* | expand |
Hello, On Tue, 5 Feb 2019 22:15:44 +0100 Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote: > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com> > > Commit 32bec8ee2fb00c6750fa842bbb0eb79b0c081fa2 > ("toolchain-external: copy ld*.so* for all C libraries") changed (among > other things) the glob pattern to catch the dynamic loader from > ld*.so.* > to > ld*.so* > > thus now matching files like 'ld-2.20.so' in addition to files like > 'ld.so.1'. I'm curious what is the problem with matching ld-2.20.so, and how can this bring more files than needed ? (Note: I think the change is OK, I just want to understand how you noticed.) Thanks! Thomas
Hi Thomas, El vie., 15 feb. 2019 a las 22:53, Thomas Petazzoni (<thomas.petazzoni@bootlin.com>) escribió: > > Hello, > > On Tue, 5 Feb 2019 22:15:44 +0100 > Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote: > > > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com> > > > > Commit 32bec8ee2fb00c6750fa842bbb0eb79b0c081fa2 > > ("toolchain-external: copy ld*.so* for all C libraries") changed (among > > other things) the glob pattern to catch the dynamic loader from > > ld*.so.* > > to > > ld*.so* > > > > thus now matching files like 'ld-2.20.so' in addition to files like > > 'ld.so.1'. > > I'm curious what is the problem with matching ld-2.20.so, and how can > this bring more files than needed ? > > (Note: I think the change is OK, I just want to understand how you > noticed.) In the common case, you have a structure like this: -rwxr-xr-x 1 tdescham tdescham 834364 Feb 16 21:23 output/target/lib/ld-2.16.so lrwxrwxrwx 1 tdescham tdescham 10 Feb 16 21:23 output/target/lib/ld.so.1 -> ld-2.16.so So, a symlink 'ld.so.1' which points to another file. Applications would have 'ld.so.1' (the link) encoded as program interpreter (readelf -l <program>, see INTERP entry) The patterns like 'ld*.so*' are passed as argument to copy_toolchain_lib_root which is defined in toolchain/helpers.mk. This macro copy_toolchain_lib_root will find all files/links matching the pattern. If a match is a regular file, it is simply copied. If it is a symbolic link, the link is copied and then the logic is recursively repeated on the link destination. That destination could either again be a link or a regular file. In the first case we recurse again, in the latter we stop and continue with the next match of the pattern. The problem this patch is solving is when a toolchain does not have this structure with a link and a real file, but rather two actual files: -rwxr-xr-x 1 tdescham tdescham 170892 Feb 16 21:55 output/target/lib/ld-2.20.so -rwxr-xr-x 1 tdescham tdescham 170892 Feb 16 21:55 output/target/lib/ld.so.1 In this case the pattern 'ld*.so*' would find two regular file matches and copy both. On the other hand, the pattern 'ld*.so.*' would only find the 'ld.so.1' file and copy just that. This saves about 170K in rootfs size. Closer inspection reveals that this particular toolchain has more such dedoubled symbolic links, e.g. the standard pattern of 'usr/lib/libfoo.so -> libfoo.so.1 -> libfoo.so.1.0.2' is not present, and each of these three components are real files. In this particular case the toolchain is from Freescale, but I don't know if it is Freescale directly or another company providing toolchains on behalf of Freescale (I'm not directly involved with this board). In any case, it is now obvious that the toolchain itself is 'broken'. Which could mean we shouldn't handle this special case in Buildroot. Even if we fixed this problem of ld.so, there would still be duplication in e.g. libstdc++.so.6 / libstdc++.so.6.0.20. And for my case specifically, we are no longer using this toolchain so I don't really need it anymore. On the other hand, I don't know if there are legitimate toolchains where this pattern occurs. Moreover, the more restrictive pattern 'ld*.so.*' is all we really need, so applying this patch also does not hurt. So, I leave it up to you :-) If we choose to apply, I can extend the commit message with some of the info above. Best regards, Thomas
Hello, On Tue, 5 Feb 2019 22:15:44 +0100 Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote: > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com> > > Commit 32bec8ee2fb00c6750fa842bbb0eb79b0c081fa2 > ("toolchain-external: copy ld*.so* for all C libraries") changed (among > other things) the glob pattern to catch the dynamic loader from > ld*.so.* > to > ld*.so* > > thus now matching files like 'ld-2.20.so' in addition to files like > 'ld.so.1'. > > However, there is no apparent reason why that change was made. It is not > explicitly mentioned in the commit message as to why that would be needed, > nor is clear based on the rest of the changes in that commit. > > The consequence of the change is that more files than needed end up in the > target root filesystem, adding extra unnecessary size to it. > > Therefore, revert the glob pattern back to what it was. > > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com> Thanks Thomas! As you know, we like this kind of patch to bitrot for a few months in patchwork, to finally apply them. So this is what just happened: I applied your patch, after significantly extending the commit log with additional explanations from your lengthy reply in this thread. Thanks for your patience! Thomas
On Sun, Oct 27, 2019, 14:59 Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > Hello, > > On Tue, 5 Feb 2019 22:15:44 +0100 > Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote: > > > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com> > > > > Commit 32bec8ee2fb00c6750fa842bbb0eb79b0c081fa2 > > ("toolchain-external: copy ld*.so* for all C libraries") changed (among > > other things) the glob pattern to catch the dynamic loader from > > ld*.so.* > > to > > ld*.so* > > > > thus now matching files like 'ld-2.20.so' in addition to files like > > 'ld.so.1'. > > > > However, there is no apparent reason why that change was made. It is not > > explicitly mentioned in the commit message as to why that would be > needed, > > nor is clear based on the rest of the changes in that commit. > > > > The consequence of the change is that more files than needed end up in > the > > target root filesystem, adding extra unnecessary size to it. > > > > Therefore, revert the glob pattern back to what it was. > > > > Signed-off-by: Thomas De Schampheleire < > thomas.de_schampheleire@nokia.com> > > Thanks Thomas! As you know, we like this kind of patch to bitrot for a > few months in patchwork, to finally apply them. > > So this is what just happened: I applied your patch, after > significantly extending the commit log with additional explanations > from your lengthy reply in this thread. > > Thanks for your patience! > Thanks :-) Best regards, Thomas
diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk index db3570d96f..e55cca539c 100644 --- a/toolchain/toolchain-external/pkg-toolchain-external.mk +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk @@ -112,7 +112,7 @@ endif # Definitions of the list of libraries that should be copied to the target. # -TOOLCHAIN_EXTERNAL_LIBS += ld*.so* libgcc_s.so.* libatomic.so.* +TOOLCHAIN_EXTERNAL_LIBS += ld*.so.* libgcc_s.so.* libatomic.so.* ifeq ($(BR2_TOOLCHAIN_EXTERNAL_GLIBC)$(BR2_TOOLCHAIN_EXTERNAL_UCLIBC),y) TOOLCHAIN_EXTERNAL_LIBS += libc.so.* libcrypt.so.* libdl.so.* libm.so.* libnsl.so.* libresolv.so.* librt.so.* libutil.so.*