From owner-dev-commits-src-all@freebsd.org Sun Jan 3 14:30:02 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 99AFB4D34D6; Sun, 3 Jan 2021 14:30:02 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-ot1-f43.google.com (mail-ot1-f43.google.com [209.85.210.43]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4D81PQ3pYSz3F7c; Sun, 3 Jan 2021 14:30:02 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-ot1-f43.google.com with SMTP id j12so23693010ota.7; Sun, 03 Jan 2021 06:30:02 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:reply-to :from:date:message-id:subject:to:cc:content-transfer-encoding; bh=zDLWU6HsHKc4Bb8f5+qQ/G01b/5pRzQqmrearPI0jOw=; b=Wtm/jlVWfyY5Uunoic+aXv+cXCIsLpk8aLrqWoPien60msH6Q3vP+QP0tCrCIAW82T Mpdoqq16J8YbBKybc3CfZvGh8c72woEBJI3phSGAtMeirTdCEUtsIVVAmxT93f87hV1/ 8im9NVkVo62EL9KWhl7cwwYofgegbuDZFxnphNuvaXbeBfeUIEHPatr8fEh919quZ7ot +OEaNCmah72jB7Lc5iKjocnk4SXTAsCVleqEOxU491TG+NgsuLRXegqYv+WLuNgvBJAq MjJ2jjWcErQiitY9mAP69HInd4+YH18qGRSrqY6kM/oBZ40y/2S47X0rRwtokQ8MxdPn u+hQ== X-Gm-Message-State: AOAM531n484Z+TOoP0KURaEGDTCCY7/fpnHj6sKkVccP04tL/KH92V/1 LcZW4NL0Q8jo1F+fB8lZhDtYi4/CpPg= X-Google-Smtp-Source: ABdhPJyyfMdjcqXhF5B1hY4WNMchK19EkEMORZXJMrSlNfJ23T07aZBWBYIkOfZW3v4GsfTUXWVabw== X-Received: by 2002:a9d:64c1:: with SMTP id n1mr48916066otl.60.1609684200954; Sun, 03 Jan 2021 06:30:00 -0800 (PST) Received: from mail-oi1-f181.google.com (mail-oi1-f181.google.com. [209.85.167.181]) by smtp.gmail.com with ESMTPSA id u76sm11231002oia.48.2021.01.03.06.30.00 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 03 Jan 2021 06:30:00 -0800 (PST) Received: by mail-oi1-f181.google.com with SMTP id l200so29154835oig.9; Sun, 03 Jan 2021 06:30:00 -0800 (PST) X-Received: by 2002:aca:e007:: with SMTP id x7mr15179451oig.8.1609684200645; Sun, 03 Jan 2021 06:30:00 -0800 (PST) MIME-Version: 1.0 References: <202012251556.0BPFu9l7029902@gitrepo.freebsd.org> In-Reply-To: Reply-To: cem@freebsd.org From: Conrad Meyer Date: Sun, 3 Jan 2021 06:29:49 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: git: f7cd7fe51c41 - sys/contrib/zstd: Import zstd 1.4.8 To: =?UTF-8?Q?Ulrich_Sp=C3=B6rlein?= Cc: src-committers , dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4D81PQ3pYSz3F7c X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; TAGGED_FROM(0.00)[] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 Jan 2021 14:30:02 -0000 On Sun, Jan 3, 2021 at 4:37 AM Ulrich Sp=C3=B6rlein = wrote: > This broke the kernel-toolchain CI on linux and macos systems, most > importantly the Github CI actions. They were nerfed due to the master > -> main switch, but I've bisected this to the merge commit that > introduces the build failure. Thanks for investigating it. > sys/contrib/zstd/lib/compress/zstd_compress_internal.h ends like so > with this import: > +/** ZSTD_cycleLog() : > + * condition for correct operation : hashLog > 1 */ > +#ifdef __FreeBSD__ /* This symbol is needed by dll-linked > CLI zstd(1). */ > +ZSTDLIB_API U32 ZSTD_cycleLog(U32 hashLog, ZSTD_strategy strat); > +#endif > > Why would this be specific to FreeBSD as the host OS? In short: it isn't (wasn't). The intention was to mark the FreeBSD-specific modification. (The modification was not the definition itself, which is present in 1.4.5, but the ZSTDLIB_API annotation.) At the time, the tree was not built on non-FreeBSD platforms. It was introduced in the v1.4.5 import: 37f1f2684f26 (r361426), not v1.4.8. The intention was to expose the symbol in our libprivatezstd.so, so that zstd(1) could link it. Upstream libzstd builds with -fvisibility=3Dhidden, and ZSTDLIB_API is __attribute__((visibility("default"))) on GCC-like platforms (i.e., Clang). Why does this show up in 1.4.8? I'm not sure. In 1.4.5, the symbol was only defined in libzstd, but never used. However, it was used in zstd(1). In 1.4.8, it is also used in libzstd: both in zstdmt_compress.c and in zstd_compress.c (though, after the definition). So apparently our non-FreeBSD CI build either skips zstd(1), or allows invoking implicit undeclared functions in zstd(1). But not in libzstd. Despite it being a private symbol, zstd(1) uses it for the flag mentioned in the 1.4.5 commit (and in v1.4.5: *only* in zstd(1)): $ nm -D /usr/bin/zstd | grep cycleLog U ZSTD_cycleLog I.e., upstream zstd build broke shared-linked zstd(1) in 1.4.5 (or maybe earlier, but this was a new breakage for us). See https://github.com/facebook/zstd/issues/1680 , https://github.com/facebook/zstd/issues/1854 . See also: https://github.com/facebook/zstd/blob/f2ac2b7bcf6294f1c3a8d62b5f2ca78cfc6fb= 980/programs/Makefile#L291-L302 Now, we do not actually appear to build libprivatezstd with -fprivate, so in fact symbols declared in zstd_compress_intenal.h without ZSTDLIB_API are all visible anyway. E.g., $ nm -D /usr/lib/libprivatezstd.so.5 | grep ZSTD_writeLastEmptyBlock 000000000001ca60 T ZSTD_writeLastEmptyBlock But maybe it would be nice to restore -fvisibility=3Dhidden to match upstream eventually. In the short term, I think this should fix CI: --- a/sys/contrib/zstd/lib/compress/zstd_compress_internal.h +++ b/sys/contrib/zstd/lib/compress/zstd_compress_internal.h @@ -1198,8 +1198,9 @@ /** ZSTD_cycleLog() : * condition for correct operation : hashLog > 1 */ -#ifdef __FreeBSD__ /* This symbol is needed by dll-linked CLI zstd(1).= */ -ZSTDLIB_API U32 ZSTD_cycleLog(U32 hashLog, ZSTD_strategy strat); -#endif +/* Begin FreeBSD - This symbol is needed by dll-linked CLI zstd(1). */ +ZSTDLIB_API +/* End FreeBSD */ +U32 ZSTD_cycleLog(U32 hashLog, ZSTD_strategy strat); #endif /* ZSTD_COMPRESS_H */ Best, Conrad