[PATCH 0/3] Fix ld.lld-17 libctf version script symbol not defined errors

[PATCH 0/3] Fix ld.lld-17 libctf version script symbol not defined errors

[Nicholas Vinson]

Starting with ld.lld-17, ld.lld is invoked with the option
--no-undefined-version enabled by default. This causes ld.lld to error whenever
a versioned symbol map contains undefined symbols. As a result, ld.lld emits
errors when building libctf.

Together these patches fix the undefined symbol errors that occur when building
libctf.

Special thanks to Sam Jones and Nick Alcock for their assitance in creating this
patch set.

Fixes Gentoo bug 914640 (https://bugs.gentoo.org/914640)

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>

Nicholas Vinson (3):
  libctf: Remove undefined functions from ver. map
  libctf: Add comment for conditionally def'd sym
  libctf: Regnerate configure

 libctf/configure    | 3 ++-
 libctf/configure.ac | 3 ++-
 libctf/libctf.ver   | 5 +----
 3 files changed, 5 insertions(+), 6 deletions(-)

-- 
2.43.2

[PATCH 1/3] libctf: Remove undefined functions from ver. map

[Nicholas Vinson]

Starting with ld.lld-17, ld.lld is invoked with the option
--no-undefined-version enabled by default. Furthermore, The functions
ctf_label_set() and ctf_label_get() are not defined. Their inclusion in
libctf/libctf.ver causes ld.lld-17 to fail emitting the following error
messages:

ld.lld: error: version script assignment of 'LIBCTF_1.0' to symbol 'ctf_label_set' failed: symbol not defined
ld.lld: error: version script assignment of 'LIBCTF_1.0' to symbol 'ctf_label_get' failed: symbol not defined

This patch fixes the issue by removing the symbol names from
libctf/libctf.ver.

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
 libctf/libctf.ver | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/libctf/libctf.ver b/libctf/libctf.ver
index c59847d012b..a685c4e3b9f 100644
--- a/libctf/libctf.ver
+++ b/libctf/libctf.ver
@@ -80,9 +80,6 @@ LIBCTF_1.0 {
 	ctf_enum_name;
 	ctf_enum_value;
 
-	ctf_label_set;
-	ctf_label_get;
-
 	ctf_label_topmost;
 	ctf_label_info;
 
-- 
2.43.2

[PATCH 2/3] libctf: Add comment for conditionally def'd sym

[Nicholas Vinson]

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(-)

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"
 
diff --git a/libctf/libctf.ver b/libctf/libctf.ver
index a685c4e3b9f..990251395ab 100644
--- a/libctf/libctf.ver
+++ b/libctf/libctf.ver
@@ -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;
-- 
2.43.2

[PATCH 3/3] libctf: Regnerate configure

[Nicholas Vinson]

Previous commit made changes to libctf/configure.ac. This commit ensures
libctf/configure is also updated so the libctf/configure.ac changes take
affect.

Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
---
 libctf/configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libctf/configure b/libctf/configure
index 3701bd8e796..df41015bf6e 100755
--- a/libctf/configure
+++ b/libctf/configure
@@ -16964,7 +16964,8 @@ int ctf_foo (void) { return 0; }
 				    int main (void) { return ctf_foo(); }
 _ACEOF
 if ac_fn_c_try_link "$LINENO"; then :
-  ac_cv_libctf_version_script="-Wl,--version-script='$srcdir/libctf.ver'"
+  ac_cv_libctf_version_script="-Wl,--version-script"
+		   decommented_version_script=t
 fi
 rm -f core conftest.err conftest.$ac_objext \
     conftest$ac_exeext conftest.$ac_ext
-- 
2.43.2

Re: [PATCH 2/3] libctf: Add comment for conditionally def'd sym

[Nick Alcock]

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
-- 
2.43.0.272.gce700b77fd

Re: [PATCH 2/3] libctf: Add comment for conditionally def'd sym

[Nicholas Vinson]

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

Re: [PATCH 2/3] libctf: Add comment for conditionally def'd sym

[Nick Alcock]

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).

-- 
NULL && (void)

Re: [PATCH 2/3] libctf: Add comment for conditionally def'd sym

[Nicholas Vinson]

On 3/4/24 08:57, Nick Alcock wrote:
> 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).

I really think using the "horrendous hack" is the better choice. ld.bfd 
and ld.lld both support the --undefined-version flag which also solves 
the problem. Unfortunately, only recent versions of ld.bfd support the 
flag. This means, of course, configure.ac would need a check to see if 
the linker supports the flag before binutils tries to use it.

Furthermore, I don't have any way of testing my changes against Solaris 
linker(s), so I would like to minimize the changes I make to 
Solaris-specific checks.

Thanks,
Nicholas Vinson

> 
>>> 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).
> 

Re: [PATCH 2/3] libctf: Add comment for conditionally def'd sym

[Nick Alcock]

On 5 Mar 2024, Nicholas Vinson spake thusly:

> On 3/4/24 08:57, Nick Alcock wrote:
>> 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).
>
> I really think using the "horrendous hack" is the better choice. ld.bfd and ld.lld both support the --undefined-version flag which
> also solves the problem. Unfortunately, only recent versions of ld.bfd support the flag. This means, of course, configure.ac would
> need a check to see if the linker supports the flag before binutils tries to use it.
>
> Furthermore, I don't have any way of testing my changes against Solaris linker(s), so I would like to minimize the changes I make to
> Solaris-specific checks.

OK, I'll whip that up shortly -- as soon as I've got some *other*
backlogged stuff in, and also figured out the cause of that ldlex_wild
problem I believe you ran into earlier, since it's now biting all my
environments and seems to be fairly easy to trigger. :(

Re: [PATCH 1/3] libctf: Remove undefined functions from ver. map

[Nick Alcock]

On 2 Mar 2024, Nicholas Vinson uttered the following:

> Starting with ld.lld-17, ld.lld is invoked with the option
> --no-undefined-version enabled by default. Furthermore, The functions
> ctf_label_set() and ctf_label_get() are not defined. Their inclusion in
> libctf/libctf.ver causes ld.lld-17 to fail emitting the following error
> messages:
>
> ld.lld: error: version script assignment of 'LIBCTF_1.0' to symbol 'ctf_label_set' failed: symbol not defined
> ld.lld: error: version script assignment of 'LIBCTF_1.0' to symbol 'ctf_label_get' failed: symbol not defined
>
> This patch fixes the issue by removing the symbol names from
> libctf/libctf.ver.

I'm throwing this into my latest series. The other stuff is biting me
too, so I clearly have to do *something* about it. Just doing the
version-script excision kludgery unconditionally does seem easiest. I'll
credit you...