Re: CVS commit: src

[Rin Okuyama <rokuyama.rk%gmail.com@localhost>]

On 2020/11/11 1:50, nia wrote:
> On Tue, Nov 10, 2020 at 04:29:21PM +0000, Taylor R Campbell wrote:
> > > Module Name:    src
> > > Committed By:   nia
> > > Date:           Sun Nov  8 21:56:48 UTC 2020
> > >
> > > Modified Files:
> > >          src/external/bsd/kyua-cli: Makefile.inc
> > >          src/external/ibm-public/postfix: Makefile.inc
> > >          src/external/public-domain/sqlite: Makefile.inc
> > >          src/external/public-domain/sqlite/bin: Makefile
> > >          src/external/public-domain/sqlite/lib: Makefile sqlite3.pc.in
> > >          src/usr.sbin/makemandb: Makefile
> > >
> > > Log Message:
> > > sqlite: do not build without multithreading support
> > >
> > > at least a few pkgsrc packages avoid base sqlite because it fails
> > > this check, and it's probably a surprising performance penalty for
> > > unsuspecting users
> >
> > Why do we even expose NetBSD's libsqlite3.so to pkgsrc?  Why not just
> > have pkgsrc always use pkgsrc sqlite3, and change base to go back to
> > the smaller non-threaded sqlite3 with only the options the things in
> > base actually need?
> >
> > Also: What is the performance penalty?  The sqlite3 documentation
> > (https://www.sqlite.org/faq.html#q6) suggests that _enabling_
> > SQLITE_THREADSAFE has a performance penalty, not the other way around.
> >
> > It seems to me we've had a lot of problems over the years trying to
> > use base sqlite3 for everything in pkgsrc.  It's not clear to me that
> > any of the recent changes are helpful for the three things in base
> > that need sqlite3, and it's also not clear that they will help with
> > running newer pkgsrc on older NetBSD.
>
> libevent is also problematic. Maybe there's just too many third-party
> libraries that are exposed that sholdn't be.

A failure for cvs, suspected to be a fallout of threaded sqlite3 was
reported in port-arm@. See thread starts with:

http://mail-index.netbsd.org/port-arm/2020/11/12/msg007046.html

Also, I encountered another fallout: ``su -'' receives SIGSEGV when exits
(on aarch64, but I bet this is MI problem):

| $ su
| # exit
| $ su -
| # exit
| [2]   Segmentation fault (core dumped) su -

The backtrace reads:

| (gdb) bt
| #0  0x0000fc2961bca308 in ?? ()
| #1  0x0000fc2961b9ec10 in __deregister_frame_info_bases ()
|    from /usr/lib/libgcc_s.so.1
| #2  0x0000fffff88425b4 in _rtld_call_function_void (obj=0xfc2962917400,
|     addr=277254663434168) at /usr/src/libexec/ld.elf_so/rtld.h:514
| ...

The fault address is anon memory w/o exec permission when su exits:

| # pmap -p xxx
| ...
| 0000FC2961BB1000      4K read/write        /altroot/lib/libgcc_s.so.1.0
| 0000FC2961BB6000     40K read/write          [ anon ]
| 0000FC2961BE6000     40K read/write          [ anon ]
| ...

However, when su is running, libpthread is loaded at this address:

| # pmap -p xxx
| ...
| 0000FC2961BB1000      4K read/write        /altroot/lib/libgcc_s.so.1.0
| 0000FC2961BB6000     40K read/write          [ anon ]
| 0000FC2961BC0000     76K read/exec         /altroot/lib/libpthread.so.1.4
| 0000FC2961BD3000     64K                   /altroot/lib/libpthread.so.1.4
| ...

su is not explicitly linked to libpthread, but it is loaded via libkrb5
via libpam. It registers its own deconstructor, however, it is unloaded
before su exits. Then, SIGSEGV takes place as a result.

IMO, this commit should be reverted ASAP. This kind of change needs more
tests, and discussion in appropriate lists before commit. Unfortunately,
the committer did not even carry out ``build.sh'' before commit (martin
fixed the build failure due to this). This is not what we, at least, I
expect our developers.

Thanks,
rin