Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Mon, Nov 11, 2019 at 8:36 AM Adrian Bunk wrote: > On Mon, Nov 11, 2019 at 07:59:43AM -0800, Khem Raj wrote: > > On Mon, 2019-11-11 at 05:04 -0800, Andre McCurdy wrote: > > > > > > With this merged, we can also drop the hack to force the sqlite > > > static > > > lib to be PIC: > > > > > > > > > > https://git.openembedded.org/openembedded-core/commit/?id=6a58e12d19c539deac9e90679a68438497a42fa4 > > > > That would be good but a caution is that if pseudo is just one usecase > > here which brought this issue forward, since we use PIE by default this > > would come up elsewhere too > > The only usecase for building a static library as PIC I am aware of is > when you want to link the static library into a shared library. > > Usually linking a static library into a shared library is a bug and > the link error is preferable since you actually intended to link > with the shared library (the linker automatically uses a static > library when it can't find a shared library). > > There are rare exceptions when this is done intentionally. > Linking a static libsqlite3.a into libpseudo.so was such an exception. > > Whether the non-PIC code is PIE or not does not make a difference. It’s not directly related to that more to the fact that we disable static libs by default and since this was available some other package might have depended on it inadvertently > > > cu > Adrian > > -- > >"Is there not promise of rain?" Ling Tan asked suddenly out > of the darkness. There had been need of rain for many days. >"Only a promise," Lao Er said. >Pearl S. Buck - Dragon Seed > > -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Mon, Nov 11, 2019 at 07:59:43AM -0800, Khem Raj wrote: > On Mon, 2019-11-11 at 05:04 -0800, Andre McCurdy wrote: > > > > With this merged, we can also drop the hack to force the sqlite > > static > > lib to be PIC: > > > > > > https://git.openembedded.org/openembedded-core/commit/?id=6a58e12d19c539deac9e90679a68438497a42fa4 > > That would be good but a caution is that if pseudo is just one usecase > here which brought this issue forward, since we use PIE by default this > would come up elsewhere too The only usecase for building a static library as PIC I am aware of is when you want to link the static library into a shared library. Usually linking a static library into a shared library is a bug and the link error is preferable since you actually intended to link with the shared library (the linker automatically uses a static library when it can't find a shared library). There are rare exceptions when this is done intentionally. Linking a static libsqlite3.a into libpseudo.so was such an exception. Whether the non-PIC code is PIE or not does not make a difference. cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Mon, 11 Nov 2019 08:17:30 -0600 Mark Hatle wrote: > First (early) version of pseudo would fork for the server, there > wasn't a seperate executed server and we had all sorts of problems > with dynamic libraries being loaded and loaded from the correct paths > (due to the LD_PRELOAD). > > Moving to two process spaces, LD_PRELOAD and the pseudo executable > happened pretty early in development, but I suspect some of this is > left over from that. To be picky: pseudo *still does this*. If it can't find the server, it clears the environment and respawns it. There were problems with this for the longest time, which turned out to be because I was calling setenv to clean the environment, and bash *overrides setenv*, so if the program spawning the pseudo server was bash, things went wrong. It should actually be fixed now and work pretty well. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Mon, 2019-11-11 at 05:04 -0800, Andre McCurdy wrote: > On Sat, Nov 9, 2019 at 8:46 AM Seebs wrote: > > On Sat, 09 Nov 2019 16:30:41 + > > Richard Purdie wrote: > > > > > I did talk briefly to Mark (also cc'd) as he wrote the original > > > patch > > > and he thought it was possibly because the client was also > > > linking > > > against sqlite3 and due to the other things the client does, that > > > was > > > problematic. > > > > It *shouldn't* link against sqlite3. But! The commit in question > > refers > > to RHEL5 and LD_LIBRARY_PATH, and I think that shook loose a > > memory: > > > > I think at one point, we had a Crucial Bug Fix in sqlite3, in our > > build > > system, and if we didn't statically link, there was a risk of > > getting > > the broken version at runtime. > > > > > The client lib doesn't and the server side should behave just > > > like any > > > other linux binary afaik so we should be ok with a dynamicly > > > linked > > > sqlite3? > > > > Yes. > > > > The issue here was, I believe, not "dynamically-linked sqlite3 per > > se", > > but "dynamic linking, plus LD_LIBRARY_PATH, picking an sqlite3 > > which > > caused us specific problems". > > > > In the Yocto environment, I think we're reasonably sure that we > > always > > get a clean Yocto-built sqlite3, and that *should* be fine. > > > > So I'd say go for it, but if you see weird sqlite3 stuff that > > happens > > only very occasionally, look at this first. :P > > With this merged, we can also drop the hack to force the sqlite > static > lib to be PIC: > > > https://git.openembedded.org/openembedded-core/commit/?id=6a58e12d19c539deac9e90679a68438497a42fa4 That would be good but a caution is that if pseudo is just one usecase here which brought this issue forward, since we use PIE by default this would come up elsewhere too -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On 11/9/19 10:30 AM, Richard Purdie wrote: > On Sat, 2019-11-09 at 10:07 -0600, Seebs wrote: >> On Sat, 9 Nov 2019 15:35:59 + >> Richard Purdie wrote: >> >>> Back in 2010[1] we made pseudo statically link against sqlite3. >>> Since then the world has changed, pseudo now has separate processes >>> for the database in the server and the client and they have >>> separate linking commands. >> >> I'm not sure what that has to do with the reasons for it being >> static? Not that I remember exactly anymore, but I think we were >> hitting problems caused if pseudo was dynamically linked to sqlite, >> but I don't remember what they were. > > I did talk briefly to Mark (also cc'd) as he wrote the original patch > and he thought it was possibly because the client was also linking > against sqlite3 and due to the other things the client does, that was > problematic. First (early) version of pseudo would fork for the server, there wasn't a seperate executed server and we had all sorts of problems with dynamic libraries being loaded and loaded from the correct paths (due to the LD_PRELOAD). Moving to two process spaces, LD_PRELOAD and the pseudo executable happened pretty early in development, but I suspect some of this is left over from that. >> I'm not entirely sure I know what you mean by "separate processes for >> the database in the server and the client". I don't think >> libpseudo.so should ever do any database things at all, and it >> shouldn't need to link to sqlite. > > I was under the impression the client may once have linked against it. > I admit I didn't dive into history, just checked that it clearly > doesn't now. The client had both from above, but also for some early logging that happened before the pseudo server was executed. Both vestiges are long fone. >>> which occurs if sqlite3-native was built on a machine with glibc >>> 2.28 or later and pseudo-native is being built on glibc before >>> that. >> >> I don't think pseudo's ever been *expected* to work when built linked >> against things built against different libc. It is *way* too full of >> unholy magics that depend on knowing which libc it's using. That >> said, the pseudo server also shouldn't really need to be statically- >> linked, the only times it ever runs under pseudo, it just immediately >> tries to escape and not run that way. > > The client lib doesn't and the server side should behave just like any > other linux binary afaik so we should be ok with a dynamicly linked > sqlite3? > >>> There appears to be no easy way to avoid this other than adding a >>> copy of sqlite3 into the pseudo recipe. Given the static linking >>> doesn't seem to be required any longer due to the separate >>> processes, >>> drop that to fix those issues. >> >> I don't understand the "separate processes" thing. So far as I know, >> pseudo has never talked to sqlite from the client. I think Jason >> Wessel had experimental patches at one point to do client-local >> sqlite work to avoid the IPC overhead, but I don't think it ever got >> merged because it turns out the IPC overhead is insignificant >> compared to everything else. > > If I'm working off incorrect assumptions, is there another reason we > need a static sqlite? I have tested this patch before I posted it and > it seems fine in our test builds... > > Cheers, > > Richard > -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Sat, Nov 9, 2019 at 8:46 AM Seebs wrote: > > On Sat, 09 Nov 2019 16:30:41 + > Richard Purdie wrote: > > > I did talk briefly to Mark (also cc'd) as he wrote the original patch > > and he thought it was possibly because the client was also linking > > against sqlite3 and due to the other things the client does, that was > > problematic. > > It *shouldn't* link against sqlite3. But! The commit in question refers > to RHEL5 and LD_LIBRARY_PATH, and I think that shook loose a memory: > > I think at one point, we had a Crucial Bug Fix in sqlite3, in our build > system, and if we didn't statically link, there was a risk of getting > the broken version at runtime. > > > The client lib doesn't and the server side should behave just like any > > other linux binary afaik so we should be ok with a dynamicly linked > > sqlite3? > > Yes. > > The issue here was, I believe, not "dynamically-linked sqlite3 per se", > but "dynamic linking, plus LD_LIBRARY_PATH, picking an sqlite3 which > caused us specific problems". > > In the Yocto environment, I think we're reasonably sure that we always > get a clean Yocto-built sqlite3, and that *should* be fine. > > So I'd say go for it, but if you see weird sqlite3 stuff that happens > only very occasionally, look at this first. :P With this merged, we can also drop the hack to force the sqlite static lib to be PIC: https://git.openembedded.org/openembedded-core/commit/?id=6a58e12d19c539deac9e90679a68438497a42fa4 -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Sat, 09 Nov 2019 16:30:41 + Richard Purdie wrote: > I did talk briefly to Mark (also cc'd) as he wrote the original patch > and he thought it was possibly because the client was also linking > against sqlite3 and due to the other things the client does, that was > problematic. It *shouldn't* link against sqlite3. But! The commit in question refers to RHEL5 and LD_LIBRARY_PATH, and I think that shook loose a memory: I think at one point, we had a Crucial Bug Fix in sqlite3, in our build system, and if we didn't statically link, there was a risk of getting the broken version at runtime. > The client lib doesn't and the server side should behave just like any > other linux binary afaik so we should be ok with a dynamicly linked > sqlite3? Yes. The issue here was, I believe, not "dynamically-linked sqlite3 per se", but "dynamic linking, plus LD_LIBRARY_PATH, picking an sqlite3 which caused us specific problems". In the Yocto environment, I think we're reasonably sure that we always get a clean Yocto-built sqlite3, and that *should* be fine. So I'd say go for it, but if you see weird sqlite3 stuff that happens only very occasionally, look at this first. :P -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Sat, 2019-11-09 at 10:07 -0600, Seebs wrote: > On Sat, 9 Nov 2019 15:35:59 + > Richard Purdie wrote: > > > Back in 2010[1] we made pseudo statically link against sqlite3. > > Since then the world has changed, pseudo now has separate processes > > for the database in the server and the client and they have > > separate linking commands. > > I'm not sure what that has to do with the reasons for it being > static? Not that I remember exactly anymore, but I think we were > hitting problems caused if pseudo was dynamically linked to sqlite, > but I don't remember what they were. I did talk briefly to Mark (also cc'd) as he wrote the original patch and he thought it was possibly because the client was also linking against sqlite3 and due to the other things the client does, that was problematic. > I'm not entirely sure I know what you mean by "separate processes for > the database in the server and the client". I don't think > libpseudo.so should ever do any database things at all, and it > shouldn't need to link to sqlite. I was under the impression the client may once have linked against it. I admit I didn't dive into history, just checked that it clearly doesn't now. > > which occurs if sqlite3-native was built on a machine with glibc > > 2.28 or later and pseudo-native is being built on glibc before > > that. > > I don't think pseudo's ever been *expected* to work when built linked > against things built against different libc. It is *way* too full of > unholy magics that depend on knowing which libc it's using. That > said, the pseudo server also shouldn't really need to be statically- > linked, the only times it ever runs under pseudo, it just immediately > tries to escape and not run that way. The client lib doesn't and the server side should behave just like any other linux binary afaik so we should be ok with a dynamicly linked sqlite3? > > There appears to be no easy way to avoid this other than adding a > > copy of sqlite3 into the pseudo recipe. Given the static linking > > doesn't seem to be required any longer due to the separate > > processes, > > drop that to fix those issues. > > I don't understand the "separate processes" thing. So far as I know, > pseudo has never talked to sqlite from the client. I think Jason > Wessel had experimental patches at one point to do client-local > sqlite work to avoid the IPC overhead, but I don't think it ever got > merged because it turns out the IPC overhead is insignificant > compared to everything else. If I'm working off incorrect assumptions, is there another reason we need a static sqlite? I have tested this patch before I posted it and it seems fine in our test builds... Cheers, Richard -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Sat, 9 Nov 2019 15:35:59 + Richard Purdie wrote: > Back in 2010[1] we made pseudo statically link against sqlite3. Since > then the world has changed, pseudo now has separate processes for the > database in the server and the client and they have separate linking > commands. I'm not sure what that has to do with the reasons for it being static? Not that I remember exactly anymore, but I think we were hitting problems caused if pseudo was dynamically linked to sqlite, but I don't remember what they were. I'm not entirely sure I know what you mean by "separate processes for the database in the server and the client". I don't think libpseudo.so should ever do any database things at all, and it shouldn't need to link to sqlite. > which occurs if sqlite3-native was built on a machine with glibc 2.28 > or later and pseudo-native is being built on glibc before that. I don't think pseudo's ever been *expected* to work when built linked against things built against different libc. It is *way* too full of unholy magics that depend on knowing which libc it's using. That said, the pseudo server also shouldn't really need to be statically-linked, the only times it ever runs under pseudo, it just immediately tries to escape and not run that way. > There appears to be no easy way to avoid this other than adding a > copy of sqlite3 into the pseudo recipe. Given the static linking > doesn't seem to be required any longer due to the separate processes, > drop that to fix those issues. I don't understand the "separate processes" thing. So far as I know, pseudo has never talked to sqlite from the client. I think Jason Wessel had experimental patches at one point to do client-local sqlite work to avoid the IPC overhead, but I don't think it ever got merged because it turns out the IPC overhead is insignificant compared to everything else. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
[OE-core] [PATCH] pseudo: Drop static linking to sqlite3
Back in 2010[1] we made pseudo statically link against sqlite3. Since then
the world has changed, pseudo now has separate processes for the database
in the server and the client and they have separate linking commands.
[1]
http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=ad0ac0ecd38fc77daf42485489fccc10a5e1e3e7
The static sqlite3-native is causing us problems, in particular:
tmp/work/x86_64-linux/pseudo-native/1.9.0+gitAUTOINC+060058bb29-r0/recipe-sysroot-native/usr/lib/libsqlite3.a(sqlite3.o):(.data.rel+0xb0):
undefined reference to `fcntl64'
which occurs if sqlite3-native was built on a machine with glibc 2.28 or later
and pseudo-native is being built on glibc before that. With dyanmical linking,
libc is backwards compatible and works but with static linking it does not.
There appears to be no easy way to avoid this other than adding a copy of
sqlite3 into the pseudo recipe. Given the static linking doesn't seem to
be required any longer due to the separate processes, drop that to fix
those issues.
Signed-off-by: Richard Purdie
---
meta/conf/distro/include/no-static-libs.inc | 4
meta/recipes-devtools/pseudo/pseudo.inc | 17 ++---
2 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/meta/conf/distro/include/no-static-libs.inc
b/meta/conf/distro/include/no-static-libs.inc
index 4141ecb7654..a3a865cac45 100644
--- a/meta/conf/distro/include/no-static-libs.inc
+++ b/meta/conf/distro/include/no-static-libs.inc
@@ -15,10 +15,6 @@ DISABLE_STATIC_pn-nativesdk-libcap = ""
DISABLE_STATIC_pn-libpcap = ""
# needed by gdb
DISABLE_STATIC_pn-readline = ""
-# needed by pseudo
-DISABLE_STATIC_pn-sqlite3 = ""
-DISABLE_STATIC_pn-sqlite3-native = ""
-DISABLE_STATIC_pn-nativesdk-sqlite3 = ""
# openjade/sgml-common have build issues without static libs
DISABLE_STATIC_pn-sgml-common-native = ""
DISABLE_STATIC_pn-openjade-native = ""
diff --git a/meta/recipes-devtools/pseudo/pseudo.inc
b/meta/recipes-devtools/pseudo/pseudo.inc
index 8b349097263..7ff8e449e9a 100644
--- a/meta/recipes-devtools/pseudo/pseudo.inc
+++ b/meta/recipes-devtools/pseudo/pseudo.inc
@@ -30,23 +30,10 @@ PSEUDO_EXTRA_OPTS ?= "--enable-force-async
--without-passwd-fallback --enable-ep
# Compile for the local machine arch...
do_compile () {
-SQLITE_LDADD='$(SQLITE)/$(SQLITE_LIB)/libsqlite3.a'
- for sqlite_link_opt in $(pkg-config sqlite3 --libs --static)
- do
- case "$sqlite_link_opt" in
- -lsqlite3)
- ;;
- -l*)
- SQLITE_LDADD="${SQLITE_LDADD} ${sqlite_link_opt}"
- ;;
- *)
- ;;
- esac
- done
if [ "${SITEINFO_BITS}" = "64" ]; then
- ${S}/configure ${PSEUDO_EXTRA_OPTS} --prefix=${prefix}
--libdir=${prefix}/lib/pseudo/lib${SITEINFO_BITS} --with-sqlite-lib=${baselib}
--with-sqlite=${STAGING_DIR_TARGET}${exec_prefix} --cflags="${CFLAGS}"
--bits=${SITEINFO_BITS} --with-static-sqlite="$SQLITE_LDADD" --without-rpath
+ ${S}/configure ${PSEUDO_EXTRA_OPTS} --prefix=${prefix}
--libdir=${prefix}/lib/pseudo/lib${SITEINFO_BITS} --with-sqlite-lib=${baselib}
--with-sqlite=${STAGING_DIR_TARGET}${exec_prefix} --cflags="${CFLAGS}"
--bits=${SITEINFO_BITS} --without-rpath
else
- ${S}/configure ${PSEUDO_EXTRA_OPTS} --prefix=${prefix}
--libdir=${prefix}/lib/pseudo/lib --with-sqlite-lib=${baselib}
--with-sqlite=${STAGING_DIR_TARGET}${exec_prefix} --cflags="${CFLAGS}"
--bits=${SITEINFO_BITS} --with-static-sqlite="$SQLITE_LDADD" --without-rpath
+ ${S}/configure ${PSEUDO_EXTRA_OPTS} --prefix=${prefix}
--libdir=${prefix}/lib/pseudo/lib --with-sqlite-lib=${baselib}
--with-sqlite=${STAGING_DIR_TARGET}${exec_prefix} --cflags="${CFLAGS}"
--bits=${SITEINFO_BITS} --without-rpath
fi
oe_runmake ${MAKEOPTS}
}
--
2.20.1
--
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
