[2/3] libctf: Add comment for conditionally def'd sym
Checks
Commit Message
Removing the symbols ctf_label_set() and ctf_label_get() from
libctf/libctf.ver revealed the following errors when trying to link with
ld.lld-17:
ld.lld: error: version script assignment of 'LIBCTF_1.0' to symbol 'ctf_arc_open' failed: symbol not defined
ld.lld: error: version script assignment of 'LIBCTF_1.0' to symbol 'ctf_fdopen' failed: symbol not defined
ld.lld: error: version script assignment of 'LIBCTF_1.0' to symbol 'ctf_open' failed: symbol not defined
ld.lld: error: version script assignment of 'LIBCTF_1.0' to symbol 'ctf_bfdopen' failed: symbol not defined
ld.lld: error: version script assignment of 'LIBCTF_1.0' to symbol 'ctf_bfdopen_ctfsect' failed: symbol not defined
This patch fixes the issue by appending the comment
'/* libctf only. */' to the ctf_arc_open() entry in libctf/libctf.ver.
The other error messages point to symbols that are already properly
annotated.
Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
libctf/configure.ac | 3 ++-
libctf/libctf.ver | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
Comments
On 2 Mar 2024, Nicholas Vinson uttered the following:
> diff --git a/libctf/configure.ac b/libctf/configure.ac
> index e4e430615bd..0494a537e78 100644
> --- a/libctf/configure.ac
> +++ b/libctf/configure.ac
> @@ -258,7 +258,8 @@ AC_CACHE_CHECK([for linker versioning flags], [ac_cv_libctf_version_script],
> CFLAGS="$CFLAGS -fPIC"
> AC_LINK_IFELSE([AC_LANG_SOURCE([[int ctf_foo (void) { return 0; }
> int main (void) { return ctf_foo(); }]])],
> - [ac_cv_libctf_version_script="-Wl,--version-script='$srcdir/libctf.ver'"],
> + [ac_cv_libctf_version_script="-Wl,--version-script"
> + decommented_version_script=t],
> [])
> LDFLAGS="$old_LDFLAGS"
Not quite. The stanza you changed there is meant for GNU ld, which
supports both undefined symbols in version scripts and /* comments */
and needs none of this hackery.
We have a three-case case statement here, with the final last-ditch
attempt being -export-symbols-regex=, and we need to add another case
for "no -B local, doesn't like nonexistent symbols, supports
--version-script".
Something like the below, perhaps. (Caveat: when I test with LLVM 17 and
LDFLAGS="-fuse-ld=lld -Wl,--no-undefined-version" there is no failure to
link libctf with trunk binutils, though ld building does fail:
ld.lld: error: undefined symbol: ldlex_backup
>>> referenced by ldgram.y:860 (../../ld/ldgram.y:860)
>>> ldgram.o:(yyparse)
>>> referenced by ldgram.y:1125 (../../ld/ldgram.y:1125)
>>> ldgram.o:(yyparse)
>>> referenced by ldgram.y:1146 (../../ld/ldgram.y:1146)
>>> ldgram.o:(yyparse)
>>> referenced 1 more times
ld.lld: error: undefined symbol: ldlex_wild
so this is not really tested and all I can really say is that clang and
lld are still happy to link. Don't trust what I wrote here, please test
it out -- and obviously it still needs things like the removal of the
ctf_label_set symbols that genuinely don't exist, as well. That patch is
fine.)
diff --git a/libctf/configure.ac b/libctf/configure.ac
index e4e430615bd..28f63792826 100644
--- a/libctf/configure.ac
+++ b/libctf/configure.ac
@@ -251,7 +251,7 @@ AC_SUBST(HAVE_TCL_TRY)
# Use a version script, if possible, or an -export-symbols-regex otherwise.
decommented_version_script=
AC_CACHE_CHECK([for linker versioning flags], [ac_cv_libctf_version_script],
- [echo 'FOO { global: mai*; local: ctf_fo*; };' > conftest.ver
+ [echo 'FOO { global: mai*; nonexistent; local: ctf_fo*; };' > conftest.ver
old_LDFLAGS="$LDFLAGS"
old_CFLAGS="$CFLAGS"
LDFLAGS="$LDFLAGS -shared -Wl,--version-script=conftest.ver"
@@ -262,7 +262,10 @@ AC_CACHE_CHECK([for linker versioning flags], [ac_cv_libctf_version_script],
[])
LDFLAGS="$old_LDFLAGS"
+ # Solaris: -B local, nonexistent symbols prohibited: use preprocessed
+ # version scripts
if test -z "$ac_cv_libctf_version_script"; then
+ echo 'FOO { global: mai*; local: ctf_fo*; };' > conftest.ver
LDFLAGS="$LDFLAGS -shared -Wl,-B,local -Wl,-z,gnu-version-script=conftest.ver"
AC_LINK_IFELSE([AC_LANG_SOURCE([[int ctf_foo (void) { return 0; }
int main (void) { return ctf_foo(); }]])],
@@ -273,6 +276,19 @@ AC_CACHE_CHECK([for linker versioning flags], [ac_cv_libctf_version_script],
fi
CFLAGS="$old_CFLAGS"
+ # LLD with --no-undefined-version on by default: no -B local, nonexistent
+ # symbols prohibited: same solution as Solaris
+ if test -z "$ac_cv_libctf_version_script"; then
+ LDFLAGS="$LDFLAGS -shared -Wl,--version-script=conftest.ver"
+ AC_LINK_IFELSE([AC_LANG_SOURCE([[int ctf_foo (void) { return 0; }
+ int main (void) { return ctf_foo(); }]])],
+ [ac_cv_libctf_version_script="-Wl,--version-script"
+ decommented_version_script=t],
+ [])
+ LDFLAGS="$old_LDFLAGS"
+ fi
+ CFLAGS="$old_CFLAGS"
+
if test -z "$ac_cv_libctf_version_script"; then
ac_cv_libctf_version_script='-export-symbols-regex ctf_.*'
fi
diff --git a/libctf/libctf.ver b/libctf/libctf.ver
index c59847d012b..474852c2c84 100644
--- a/libctf/libctf.ver
+++ b/libctf/libctf.ver
@@ -139,7 +139,6 @@ LIBCTF_1.0 {
ctf_arc_write;
ctf_arc_write_fd;
- ctf_arc_open;
ctf_arc_bufopen;
ctf_arc_close;
ctf_arc_open_by_name;
@@ -165,6 +164,7 @@ LIBCTF_1.0 {
ctf_link_shuffle_syms;
ctf_link_write;
+ ctf_arc_open; /* libctf only. */
ctf_fdopen; /* libctf only. */
ctf_open; /* libctf only. */
ctf_bfdopen; /* libctf only. */
base-commit: 90f8d97c8efa75f7f019b868eca9c626bc35203d
On 3/3/24 10:10, Nick Alcock wrote:
> On 2 Mar 2024, Nicholas Vinson uttered the following:
>
>> diff --git a/libctf/configure.ac b/libctf/configure.ac
>> index e4e430615bd..0494a537e78 100644
>> --- a/libctf/configure.ac
>> +++ b/libctf/configure.ac
>> @@ -258,7 +258,8 @@ AC_CACHE_CHECK([for linker versioning flags], [ac_cv_libctf_version_script],
>> CFLAGS="$CFLAGS -fPIC"
>> AC_LINK_IFELSE([AC_LANG_SOURCE([[int ctf_foo (void) { return 0; }
>> int main (void) { return ctf_foo(); }]])],
>> - [ac_cv_libctf_version_script="-Wl,--version-script='$srcdir/libctf.ver'"],
>> + [ac_cv_libctf_version_script="-Wl,--version-script"
>> + decommented_version_script=t],
>> [])
>> LDFLAGS="$old_LDFLAGS"
>
> Not quite. The stanza you changed there is meant for GNU ld, which
> supports both undefined symbols in version scripts and /* comments */
> and needs none of this hackery.
>
> We have a three-case case statement here, with the final last-ditch
> attempt being -export-symbols-regex=, and we need to add another case
> for "no -B local, doesn't like nonexistent symbols, supports
> --version-script".
The three cases are not fully independent. The first case defines
conftest.ver and conftest.c in such a way that when the test is run both
ld.bfd and ld.lld pass. As a result, ac_cv_libctf_version_script is set.
Then the next check is if ac_cv_libctf_version_script is empty. It's
not, so the check with '-Wl,-B,local' is never run.
conftest.ver and/or conftest.c could be updated so the test passes with
ld.bfd and not ld.lld, in which case another block is added for ld.lld.
Even though ld.bfd allows undefined symbols in the symbol version map, I
believe it excludes them in the libctf symbol table. If that's correct,
then I request this change be reconsidered because it's a much simpler
change that supports both ld.bfd and ld.lld.
>
> Something like the below, perhaps. (Caveat: when I test with LLVM 17 and
> LDFLAGS="-fuse-ld=lld -Wl,--no-undefined-version" there is no failure to
> link libctf with trunk binutils, though ld building does fail:
>
> ld.lld: error: undefined symbol: ldlex_backup
>>>> referenced by ldgram.y:860 (../../ld/ldgram.y:860)
>>>> ldgram.o:(yyparse)
>>>> referenced by ldgram.y:1125 (../../ld/ldgram.y:1125)
>>>> ldgram.o:(yyparse)
>>>> referenced by ldgram.y:1146 (../../ld/ldgram.y:1146)
>>>> ldgram.o:(yyparse)
>>>> referenced 1 more times
>
> ld.lld: error: undefined symbol: ldlex_wild
>
> so this is not really tested and all I can really say is that clang and
> lld are still happy to link. Don't trust what I wrote here, please test
> it out -- and obviously it still needs things like the removal of the
> ctf_label_set symbols that genuinely don't exist, as well. That patch is
> fine.)
I haven't seen that issue and I've been testing with clang and ld.lld
this entire time.
I wonder if this is caused by flex/bison (or lex/yacc). For the record,
I'm using flex-2.6.4 (Gentoo revision r6) and bison-3.8.2 (Gentoo
revision r2).
>
> diff --git a/libctf/configure.ac b/libctf/configure.ac
> index e4e430615bd..28f63792826 100644
> --- a/libctf/configure.ac
> +++ b/libctf/configure.ac
> @@ -251,7 +251,7 @@ AC_SUBST(HAVE_TCL_TRY)
> # Use a version script, if possible, or an -export-symbols-regex otherwise.
> decommented_version_script=
> AC_CACHE_CHECK([for linker versioning flags], [ac_cv_libctf_version_script],
> - [echo 'FOO { global: mai*; local: ctf_fo*; };' > conftest.ver
> + [echo 'FOO { global: mai*; nonexistent; local: ctf_fo*; };' > conftest.ver
> old_LDFLAGS="$LDFLAGS"
> old_CFLAGS="$CFLAGS"
> LDFLAGS="$LDFLAGS -shared -Wl,--version-script=conftest.ver"
> @@ -262,7 +262,10 @@ AC_CACHE_CHECK([for linker versioning flags], [ac_cv_libctf_version_script],
> [])
> LDFLAGS="$old_LDFLAGS"
>
> + # Solaris: -B local, nonexistent symbols prohibited: use preprocessed
> + # version scripts
> if test -z "$ac_cv_libctf_version_script"; then
> + echo 'FOO { global: mai*; local: ctf_fo*; };' > conftest.ver
> LDFLAGS="$LDFLAGS -shared -Wl,-B,local -Wl,-z,gnu-version-script=conftest.ver"
> AC_LINK_IFELSE([AC_LANG_SOURCE([[int ctf_foo (void) { return 0; }
> int main (void) { return ctf_foo(); }]])],
> @@ -273,6 +276,19 @@ AC_CACHE_CHECK([for linker versioning flags], [ac_cv_libctf_version_script],
> fi
> CFLAGS="$old_CFLAGS"
>
> + # LLD with --no-undefined-version on by default: no -B local, nonexistent
> + # symbols prohibited: same solution as Solaris
> + if test -z "$ac_cv_libctf_version_script"; then
> + LDFLAGS="$LDFLAGS -shared -Wl,--version-script=conftest.ver"
> + AC_LINK_IFELSE([AC_LANG_SOURCE([[int ctf_foo (void) { return 0; }
> + int main (void) { return ctf_foo(); }]])],
> + [ac_cv_libctf_version_script="-Wl,--version-script"
> + decommented_version_script=t],
> + [])
> + LDFLAGS="$old_LDFLAGS"
> + fi
> + CFLAGS="$old_CFLAGS"
> +
> if test -z "$ac_cv_libctf_version_script"; then
> ac_cv_libctf_version_script='-export-symbols-regex ctf_.*'
> fi
> diff --git a/libctf/libctf.ver b/libctf/libctf.ver
> index c59847d012b..474852c2c84 100644
> --- a/libctf/libctf.ver
> +++ b/libctf/libctf.ver
> @@ -139,7 +139,6 @@ LIBCTF_1.0 {
>
> ctf_arc_write;
> ctf_arc_write_fd;
> - ctf_arc_open;
> ctf_arc_bufopen;
> ctf_arc_close;
> ctf_arc_open_by_name;
> @@ -165,6 +164,7 @@ LIBCTF_1.0 {
> ctf_link_shuffle_syms;
> ctf_link_write;
>
> + ctf_arc_open; /* libctf only. */
> ctf_fdopen; /* libctf only. */
> ctf_open; /* libctf only. */
> ctf_bfdopen; /* libctf only. */
>
> base-commit: 90f8d97c8efa75f7f019b868eca9c626bc35203d
On 3 Mar 2024, Nicholas Vinson said:
> On 3/3/24 10:10, Nick Alcock wrote:
>> On 2 Mar 2024, Nicholas Vinson uttered the following:
>>
>>> diff --git a/libctf/configure.ac b/libctf/configure.ac
>>> index e4e430615bd..0494a537e78 100644
>>> --- a/libctf/configure.ac
>>> +++ b/libctf/configure.ac
>>> @@ -258,7 +258,8 @@ AC_CACHE_CHECK([for linker versioning flags], [ac_cv_libctf_version_script],
>>> CFLAGS="$CFLAGS -fPIC"
>>> AC_LINK_IFELSE([AC_LANG_SOURCE([[int ctf_foo (void) { return 0; }
>>> int main (void) { return ctf_foo(); }]])],
>>> - [ac_cv_libctf_version_script="-Wl,--version-script='$srcdir/libctf.ver'"],
>>> + [ac_cv_libctf_version_script="-Wl,--version-script"
>>> + decommented_version_script=t],
>>> [])
>>> LDFLAGS="$old_LDFLAGS"
>> Not quite. The stanza you changed there is meant for GNU ld, which
>> supports both undefined symbols in version scripts and /* comments */
>> and needs none of this hackery.
>> We have a three-case case statement here, with the final last-ditch
>> attempt being -export-symbols-regex=, and we need to add another case
>> for "no -B local, doesn't like nonexistent symbols, supports
>> --version-script".
>
> The three cases are not fully independent. The first case defines conftest.ver and conftest.c in such a way that when the test is
> run both ld.bfd and ld.lld pass. As a result, ac_cv_libctf_version_script is set.
Yep.
> Then the next check is if ac_cv_libctf_version_script is empty. It's not, so the check with '-Wl,-B,local' is never run.
That's why I added 'nonexistent' to the version script at that point, to
ensure that if the linker objects to nonexistent symbols in the version
script, we fail that test, leaving ac_cv_libctf_version_script unset. So
the recent-lld setup fails that, cascades into the Solaris version,
fails that, then hits the next case, freshly added, and passes it.
> Even though ld.bfd allows undefined symbols in the symbol version map,
> I believe it excludes them in the libctf symbol table. If that's
> correct, then I request this change be reconsidered because it's a
> much simpler change that supports both ld.bfd and ld.lld.
The *test* is simpler, but it expands use of the horrendous hack which
involves sed-transforming the version script. I was trying to minimize
the number of cases in which we have to use that... but I guess if it's
increasingly a lost cause to do so we could move to doing the
sed-transformation everywhere that supports version scripts at all. It
would certainly simplify things a bit (though not completely simplify
this away, since Solaris still needs different flags as well).
>> Something like the below, perhaps. (Caveat: when I test with LLVM 17 and
>> LDFLAGS="-fuse-ld=lld -Wl,--no-undefined-version" there is no failure to
>> link libctf with trunk binutils, though ld building does fail:
>> ld.lld: error: undefined symbol: ldlex_backup
>>>>> referenced by ldgram.y:860 (../../ld/ldgram.y:860)
>>>>> ldgram.o:(yyparse)
>>>>> referenced by ldgram.y:1125 (../../ld/ldgram.y:1125)
>>>>> ldgram.o:(yyparse)
>>>>> referenced by ldgram.y:1146 (../../ld/ldgram.y:1146)
>>>>> ldgram.o:(yyparse)
>>>>> referenced 1 more times
>> ld.lld: error: undefined symbol: ldlex_wild
>> so this is not really tested and all I can really say is that clang and
>> lld are still happy to link. Don't trust what I wrote here, please test
>> it out -- and obviously it still needs things like the removal of the
>> ctf_label_set symbols that genuinely don't exist, as well. That patch is
>> fine.)
>
> I haven't seen that issue and I've been testing with clang and ld.lld this entire time.
>
> I wonder if this is caused by flex/bison (or lex/yacc). For the record,
> I'm using flex-2.6.4 (Gentoo revision r6) and bison-3.8.2 (Gentoo revision r2).
flex 2.6.4-57 here, but otherwise identical. ldlex_backup is of course a
symbol in ld, and is definitely defined, as is ldlex_wild. Weird.
This is with LLVM 17.0.6 (release, from git).
@@ -258,7 +258,8 @@ AC_CACHE_CHECK([for linker versioning flags], [ac_cv_libctf_version_script],
CFLAGS="$CFLAGS -fPIC"
AC_LINK_IFELSE([AC_LANG_SOURCE([[int ctf_foo (void) { return 0; }
int main (void) { return ctf_foo(); }]])],
- [ac_cv_libctf_version_script="-Wl,--version-script='$srcdir/libctf.ver'"],
+ [ac_cv_libctf_version_script="-Wl,--version-script"
+ decommented_version_script=t],
[])
LDFLAGS="$old_LDFLAGS"
@@ -136,7 +136,7 @@ LIBCTF_1.0 {
ctf_arc_write;
ctf_arc_write_fd;
- ctf_arc_open;
+ ctf_arc_open; /* libctf only. */
ctf_arc_bufopen;
ctf_arc_close;
ctf_arc_open_by_name;