[PATCH] elf/tests: Make thrlock and noload depend on libm

[PATCH] elf/tests: Make thrlock and noload depend on libm

[Siddhesh Poyarekar]

Both tests try to dlopen libm.so at runtime, so make them depend on it
to avoid running before libm.so is built.
---
 elf/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elf/Makefile b/elf/Makefile
index b1e01d9516..4b320e8b3a 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -1281,6 +1281,8 @@ tst-leaks1-ENV = MALLOC_TRACE=$(objpfx)tst-leaks1.mtrace
 tst-leaks1-static-ENV = MALLOC_TRACE=$(objpfx)tst-leaks1-static.mtrace
 
 $(objpfx)tst-thrlock: $(shared-thread-library)
+$(objpfx)tst-thrlock.out: $(libm)
+$(objpfx)tst-noload.out: $(libm)
 
 tst-tst-dlopen-tlsmodid-no-pie = yes
 $(objpfx)tst-dlopen-tlsmodid: $(shared-thread-library)
-- 
2.31.1

Re: [PATCH] elf/tests: Make thrlock and noload depend on libm

[Florian Weimer]

* Siddhesh Poyarekar via Libc-alpha:

> Both tests try to dlopen libm.so at runtime, so make them depend on it
> to avoid running before libm.so is built.

Can that really happen in the current build system?

It's still value to re-run the tests after libm has changed, so the
patch looks okay to me (only the justification is lacking).

Thanks,
Florian

Re: [PATCH] elf/tests: Make thrlock and noload depend on libm

[Siddhesh Poyarekar]

On 7/7/21 10:36 PM, Florian Weimer via Libc-alpha wrote:
> * Siddhesh Poyarekar via Libc-alpha:
> 
>> Both tests try to dlopen libm.so at runtime, so make them depend on it
>> to avoid running before libm.so is built.
> 
> Can that really happen in the current build system?

I don't the exact sequence of events, but I did run into this a few 
minutes ago, which is how I realized there was a missing dependency. 
The tests failed with "file too short", which was probably them racing 
with the static linker linking libm.so.

Siddhesh

Re: [PATCH] elf/tests: Make thrlock and noload depend on libm

[Florian Weimer]

* Siddhesh Poyarekar:

> On 7/7/21 10:36 PM, Florian Weimer via Libc-alpha wrote:
>> * Siddhesh Poyarekar via Libc-alpha:
>> 
>>> Both tests try to dlopen libm.so at runtime, so make them depend on it
>>> to avoid running before libm.so is built.
>> Can that really happen in the current build system?
>
> I don't the exact sequence of events, but I did run into this a few
> minutes ago, which is how I realized there was a missing dependency. 
> The tests failed with "file too short", which was probably them racing
> with the static linker linking libm.so.

Hmm.  I would expect that recursive make running in elf subdirectory
simply does not have the knowledge how to link libm properly.  And it
will definitely not do that while running tests.

Thanks,
Florian

Re: [PATCH] elf/tests: Make thrlock and noload depend on libm

[Siddhesh Poyarekar]

On 7/8/21 11:16 AM, Florian Weimer wrote:
> * Siddhesh Poyarekar:
> 
>> On 7/7/21 10:36 PM, Florian Weimer via Libc-alpha wrote:
>>> * Siddhesh Poyarekar via Libc-alpha:
>>>
>>>> Both tests try to dlopen libm.so at runtime, so make them depend on it
>>>> to avoid running before libm.so is built.
>>> Can that really happen in the current build system?
>>
>> I don't the exact sequence of events, but I did run into this a few
>> minutes ago, which is how I realized there was a missing dependency.
>> The tests failed with "file too short", which was probably them racing
>> with the static linker linking libm.so.
> 
> Hmm.  I would expect that recursive make running in elf subdirectory
> simply does not have the knowledge how to link libm properly.  And it
> will definitely not do that while running tests.

My mental model of how this works is that make builds dependency trees 
and then runs independent trees in parallel.  In that model, if tests 
don't have that dependency, they may end up in a different dependency 
tree from that of libm and both could then happen in parallel.  Of 
course, I don't actually know if that's how make works because I haven't 
read the sources.

Oh, and running only tests (assuming that all built objects are up to 
date) won't do this.  It'll only happen if some object needs to be 
updated when the test is run because an undercaffeinated developer ran 
make check instead of make && make check :)

Siddhesh

Re: [PATCH] elf/tests: Make thrlock and noload depend on libm

[Florian Weimer]

* Siddhesh Poyarekar:

> Oh, and running only tests (assuming that all built objects are up to
> date) won't do this.  It'll only happen if some object needs to be 
> updated when the test is run because an undercaffeinated developer ran
> make check instead of make && make check :)

If you do that, you'll likely end up with corrupt objects anyway.
I think some assert in allocatestack.c is particularly common
(or at least was when we still had a separate libpthread).  It
definitely invalidates testing.

We really need to fix that, but the dependency won't help with that.
(But as I said, adding the dependency is correct, just the commit
message is misleading.)

Thanks,
Florian

Re: [PATCH] elf/tests: Make thrlock and noload depend on libm

[Siddhesh Poyarekar]

On 7/8/21 11:39 AM, Florian Weimer via Libc-alpha wrote:
> * Siddhesh Poyarekar:
> 
>> Oh, and running only tests (assuming that all built objects are up to
>> date) won't do this.  It'll only happen if some object needs to be
>> updated when the test is run because an undercaffeinated developer ran
>> make check instead of make && make check :)
> 
> If you do that, you'll likely end up with corrupt objects anyway.
> I think some assert in allocatestack.c is particularly common
> (or at least was when we still had a separate libpthread).  It
> definitely invalidates testing.
> 
> We really need to fix that, but the dependency won't help with that.
> (But as I said, adding the dependency is correct, just the commit
> message is misleading.)

OK, I'll update the commit message to say:

Both tests try to dlopen libm.so at runtime, so make them depend on it 
so that they're executed if libm.so has been updated.

Siddhesh

Re: [PATCH] elf/tests: Make thrlock and noload depend on libm

[Florian Weimer]

* Siddhesh Poyarekar:

> On 7/8/21 11:39 AM, Florian Weimer via Libc-alpha wrote:
>> * Siddhesh Poyarekar:
>> 
>>> Oh, and running only tests (assuming that all built objects are up to
>>> date) won't do this.  It'll only happen if some object needs to be
>>> updated when the test is run because an undercaffeinated developer ran
>>> make check instead of make && make check :)
>> If you do that, you'll likely end up with corrupt objects anyway.
>> I think some assert in allocatestack.c is particularly common
>> (or at least was when we still had a separate libpthread).  It
>> definitely invalidates testing.
>> We really need to fix that, but the dependency won't help with that.
>> (But as I said, adding the dependency is correct, just the commit
>> message is misleading.)
>
> OK, I'll update the commit message to say:
>
> Both tests try to dlopen libm.so at runtime, so make them depend on it
> so that they're executed if libm.so has been updated.

Thanks. 8-)

Florian