[PATCH 2/2] Build gdb.base/gnu-ifunc.exp with lazy binding

[PATCH 2/2] Build gdb.base/gnu-ifunc.exp with lazy binding

[Luis Machado]

The test expects the ifunc resolver to run lazily, at a later stage.

Depending on the distro and toolchain configuration, this is not the
case. Some configurations use non-lazy binding and thus the ifunc resolver
resolves all the ifunc references very early in the process startup, before
main.

Ubuntu is one such case. It has switched its toolchains to pass -Wl,z,now by
default, since 16.04. This wasn't a problem before 20.04 (at least for
aarch64) because the toolchains did not support ifunc's.

Forcing lazy binding makes the test run as expected, as opposed to the 80 or
so failures it showed before the change.

Tested on aarch64-linux/x86_64-linux Ubuntu 20.04.

gdb/testsuite:

YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>

	* gdb.base/gnu-ifunc.exp (build): Pass -Wl,z,lazy.
---
 gdb/testsuite/gdb.base/gnu-ifunc.exp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
index d6064d3c4ac..4ec529130ce 100644
--- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
+++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
@@ -64,6 +64,10 @@ proc build {resolver_attr resolver_debug final_debug} {
     set lib_opts {}
     set final_opts {}
 
+    # Force lazy binding so we don't resolve everything at process startup.
+    lappend exec_opts "additional_flags=-Wl,-z,lazy"
+    lappend lib_opts "additional_flags=-Wl,-z,lazy"
+
     if {$resolver_attr} {
 	lappend lib_opts "additional_flags=-DIFUNC_RESOLVER_ATTR"
     }
-- 
2.25.1

Re: [PATCH 2/2] Build gdb.base/gnu-ifunc.exp with lazy binding

[Luis Machado]

The 2/2 info is bogus and should be ignored. I had another patch on the 
stack.

On 1/25/21 11:19 AM, Luis Machado wrote:
> The test expects the ifunc resolver to run lazily, at a later stage.
> 
> Depending on the distro and toolchain configuration, this is not the
> case. Some configurations use non-lazy binding and thus the ifunc resolver
> resolves all the ifunc references very early in the process startup, before
> main.
> 
> Ubuntu is one such case. It has switched its toolchains to pass -Wl,z,now by
> default, since 16.04. This wasn't a problem before 20.04 (at least for
> aarch64) because the toolchains did not support ifunc's.
> 
> Forcing lazy binding makes the test run as expected, as opposed to the 80 or
> so failures it showed before the change.
> 
> Tested on aarch64-linux/x86_64-linux Ubuntu 20.04.
> 
> gdb/testsuite:
> 
> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>
> 
> 	* gdb.base/gnu-ifunc.exp (build): Pass -Wl,z,lazy.
> ---
>   gdb/testsuite/gdb.base/gnu-ifunc.exp | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
> index d6064d3c4ac..4ec529130ce 100644
> --- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
> +++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
> @@ -64,6 +64,10 @@ proc build {resolver_attr resolver_debug final_debug} {
>       set lib_opts {}
>       set final_opts {}
>   
> +    # Force lazy binding so we don't resolve everything at process startup.
> +    lappend exec_opts "additional_flags=-Wl,-z,lazy"
> +    lappend lib_opts "additional_flags=-Wl,-z,lazy"
> +
>       if {$resolver_attr} {
>   	lappend lib_opts "additional_flags=-DIFUNC_RESOLVER_ATTR"
>       }
> 

Re: [PATCH 2/2] Build gdb.base/gnu-ifunc.exp with lazy binding

[Simon Marchi]

On 2021-01-25 9:19 a.m., Luis Machado via Gdb-patches wrote:
> The test expects the ifunc resolver to run lazily, at a later stage.
> 
> Depending on the distro and toolchain configuration, this is not the
> case. Some configurations use non-lazy binding and thus the ifunc resolver
> resolves all the ifunc references very early in the process startup, before
> main.
> 
> Ubuntu is one such case. It has switched its toolchains to pass -Wl,z,now by
> default, since 16.04. This wasn't a problem before 20.04 (at least for
> aarch64) because the toolchains did not support ifunc's.
> 
> Forcing lazy binding makes the test run as expected, as opposed to the 80 or
> so failures it showed before the change.
> 
> Tested on aarch64-linux/x86_64-linux Ubuntu 20.04.
> 
> gdb/testsuite:
> 
> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>
> 
> 	* gdb.base/gnu-ifunc.exp (build): Pass -Wl,z,lazy.
> ---
>  gdb/testsuite/gdb.base/gnu-ifunc.exp | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
> index d6064d3c4ac..4ec529130ce 100644
> --- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
> +++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
> @@ -64,6 +64,10 @@ proc build {resolver_attr resolver_debug final_debug} {
>      set lib_opts {}
>      set final_opts {}
>  
> +    # Force lazy binding so we don't resolve everything at process startup.
> +    lappend exec_opts "additional_flags=-Wl,-z,lazy"
> +    lappend lib_opts "additional_flags=-Wl,-z,lazy"
> +
>      if {$resolver_attr} {
>  	lappend lib_opts "additional_flags=-DIFUNC_RESOLVER_ATTR"
>      }
> 

I'm trying to see if doing this change makes the test any less
effective.  When passing special flags, there's always the risk that
because we don't test the default platform's configuration, we don't
test what most people will use in the real world.

I think that's fine here though, because what we test now appears to be
a superset of what people will encounter in the real world.  We test
what happens before and after the ifuncs are resolved, whereas
real-world users will only see the after.

So that LGTM.

Simon

Re: [PATCH 2/2] Build gdb.base/gnu-ifunc.exp with lazy binding

[Luis Machado]

On 1/25/21 1:53 PM, Simon Marchi wrote:
> On 2021-01-25 9:19 a.m., Luis Machado via Gdb-patches wrote:
>> The test expects the ifunc resolver to run lazily, at a later stage.
>>
>> Depending on the distro and toolchain configuration, this is not the
>> case. Some configurations use non-lazy binding and thus the ifunc resolver
>> resolves all the ifunc references very early in the process startup, before
>> main.
>>
>> Ubuntu is one such case. It has switched its toolchains to pass -Wl,z,now by
>> default, since 16.04. This wasn't a problem before 20.04 (at least for
>> aarch64) because the toolchains did not support ifunc's.
>>
>> Forcing lazy binding makes the test run as expected, as opposed to the 80 or
>> so failures it showed before the change.
>>
>> Tested on aarch64-linux/x86_64-linux Ubuntu 20.04.
>>
>> gdb/testsuite:
>>
>> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>
>>
>> 	* gdb.base/gnu-ifunc.exp (build): Pass -Wl,z,lazy.
>> ---
>>   gdb/testsuite/gdb.base/gnu-ifunc.exp | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
>> index d6064d3c4ac..4ec529130ce 100644
>> --- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
>> +++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
>> @@ -64,6 +64,10 @@ proc build {resolver_attr resolver_debug final_debug} {
>>       set lib_opts {}
>>       set final_opts {}
>>   
>> +    # Force lazy binding so we don't resolve everything at process startup.
>> +    lappend exec_opts "additional_flags=-Wl,-z,lazy"
>> +    lappend lib_opts "additional_flags=-Wl,-z,lazy"
>> +
>>       if {$resolver_attr} {
>>   	lappend lib_opts "additional_flags=-DIFUNC_RESOLVER_ATTR"
>>       }
>>
> 
> I'm trying to see if doing this change makes the test any less
> effective.  When passing special flags, there's always the risk that
> because we don't test the default platform's configuration, we don't
> test what most people will use in the real world.

That's true. The current test exercises only a possible path. 
Unfortunately ifunc's are a bit complex and vary considerably between 
architectures in terms of when the resolvers are invoked and how they 
are invoked.

For example, AArch64 had an ifunc resolver ABI change last year to allow 
passing a couple parameters to the resolver function.

> 
> I think that's fine here though, because what we test now appears to be
> a superset of what people will encounter in the real world.  We test
> what happens before and after the ifuncs are resolved, whereas
> real-world users will only see the after.

Right. It exercises quite a few useful things. Passing a particular 
linker option to make the architecture behave the way the test expects 
it to seems simple and effective.

Ideally we'd need to have some other tests for better covered of 
arch-specific features.

> 
> So that LGTM.

Thanks for taking a look. I'll push it later in the week, in case 
someone has any other feedback.