All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] tst_strstatus.c fails on Alpine
@ 2021-07-12 17:02 Petr Vorel
  2021-07-13  6:08 ` Jan Stancek
  0 siblings, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2021-07-12 17:02 UTC (permalink / raw)
  To: ltp

Hi all,

I see failures of lib/newlib_tests/tst_strstatus on Alpine:

tst_strstatus.c:31: TPASS: exited with 1
tst_strstatus.c:31: TPASS: killed by SIGHUP
tst_strstatus.c:31: TPASS: is stopped
tst_strstatus.c:31: TPASS: is resumed
tst_strstatus.c:29: TFAIL: killed by ??? != invalid status 0xff

Any idea what could be wrong?

Kind regards,
Petr

$ strace -ff ./lib/newlib_tests/tst_strstatus
write(2, "tst_test.c:1261: \33[1;34mTINFO: \33"..., 65tst_test.c:1261: TINFO: Timeout per run is 0h 05m 00s
) = 65
getpid()                                = 6286
setitimer(ITIMER_REAL, {it_interval={tv_sec=0, tv_usec=0}, it_value={tv_sec=300, tv_usec=0}}, {it_interval={tv_sec=0, tv_usec=0}, it_value={tv_sec=0, tv_usec=0}}) = 0
rt_sigaction(SIGINT, {sa_handler=0x557c315e0fd0, sa_mask=[], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f6b1a5b7304}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
rt_sigprocmask(SIG_BLOCK, ~[], [], 8)   = 0
fork(strace: Process 6287 attached
 <unfinished ...>
[pid  6287] gettid()                    = 6287
[pid  6287] rt_sigprocmask(SIG_SETMASK, [],  <unfinished ...>
[pid  6286] <... fork resumed>)         = 6287
[pid  6287] <... rt_sigprocmask resumed>NULL, 8) = 0
[pid  6287] rt_sigaction(SIGALRM, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f6b1a5b7304},  <unfinished ...>
[pid  6286] rt_sigprocmask(SIG_SETMASK, [],  <unfinished ...>
[pid  6287] <... rt_sigaction resumed>{sa_handler=0x557c315e1010, sa_mask=[], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f6b1a5b7304}, 8) = 0
[pid  6286] <... rt_sigprocmask resumed>NULL, 8) = 0
[pid  6287] rt_sigaction(SIGUSR1, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f6b1a5b7304},  <unfinished ...>
[pid  6286] wait4(6287,  <unfinished ...>
[pid  6287] <... rt_sigaction resumed>{sa_handler=0x557c315e0ea0, sa_mask=[], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f6b1a5b7304}, 8) = 0
[pid  6287] rt_sigaction(SIGINT, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f6b1a5b7304}, {sa_handler=0x557c315e0fd0, sa_mask=[], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f6b1a5b7304}, 8) = 0
[pid  6287] setpgid(0, 0)               = 0
[pid  6287] clock_gettime(CLOCK_REALTIME, NULL) = -1 EFAULT (Bad address)
[pid  6287] clock_gettime(CLOCK_MONOTONIC, {tv_sec=12534, tv_nsec=660628778}) = 0
[pid  6287] getppid()                   = 6286
[pid  6287] kill(6286, SIGUSR1)         = 0
[pid  6287] getpid()                    = 6287
[pid  6286] <... wait4 resumed>0x7ffc58615040, 0, NULL) = ? ERESTARTSYS (To be restarted if SA_RESTART is set)
[pid  6287] getpid()                    = 6287
[pid  6287] write(2, "tst_strstatus.c:31: \33[1;32mTPASS"..., 52tst_strstatus.c:31: TPASS: exited with 1
) = 52
[pid  6286] --- SIGUSR1 {si_signo=SIGUSR1, si_code=SI_USER, si_pid=6287, si_uid=1000} ---
[pid  6287] getpid()                    = 6287
[pid  6287] wait4(-1, 0x7ffc58614fe4, 0, NULL) = -1 ECHILD (No child process)
[pid  6286] setitimer(ITIMER_REAL, {it_interval={tv_sec=0, tv_usec=0}, it_value={tv_sec=300, tv_usec=0}}, {it_interval={tv_sec=0, tv_usec=0}, it_value={tv_sec=299, tv_usec=994322}}) = 0
[pid  6287] write(2, "tst_strstatus.c:31: \33[1;32mTPASS"..., 55tst_strstatus.c:31: TPASS: killed by SIGHUP
) = 55
[pid  6287] getpid()                    = 6287
[pid  6287] wait4(-1, 0x7ffc58614fe4, 0, NULL) = -1 ECHILD (No child process)
[pid  6287] write(2, "tst_strstatus.c:31: \33[1;32mTPASS"..., 49 <unfinished ...>
[pid  6286] rt_sigreturn({mask=[]}tst_strstatus.c:31: TPASS: is stopped
 <unfinished ...>
[pid  6287] <... write resumed>)        = 49
[pid  6286] <... rt_sigreturn resumed>) = 61
[pid  6287] getpid()                    = 6287
[pid  6287] wait4(-1, 0x7ffc58614fe4, 0, NULL) = -1 ECHILD (No child process)
[pid  6287] write(2, "tst_strstatus.c:31: \33[1;32mTPASS"..., 49 <unfinished ...>
[pid  6286] wait4(6287, tst_strstatus.c:31: TPASS: is resumed
 <unfinished ...>
[pid  6287] <... write resumed>)        = 49
[pid  6287] getpid()                    = 6287
[pid  6287] wait4(-1, 0x7ffc58614fe4, 0, NULL) = -1 ECHILD (No child process)
[pid  6287] write(2, "tst_strstatus.c:29: \33[1;31mTFAIL"..., 75tst_strstatus.c:29: TFAIL: killed by ??? != invalid status 0xff
) = 75
[pid  6287] getpid()                    = 6287
[pid  6287] wait4(-1, 0x7ffc58614fe4, 0, NULL) = -1 ECHILD (No child process)
[pid  6287] clock_gettime(CLOCK_MONOTONIC, {tv_sec=12534, tv_nsec=663302915}) = 0
[pid  6287] getppid()                   = 6286
[pid  6287] kill(6286, SIGUSR1)         = 0
[pid  6286] <... wait4 resumed>0x7ffc58615040, 0, NULL) = ? ERESTARTSYS (To be restarted if SA_RESTART is set)
[pid  6287] exit_group(0)               = ?


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [LTP] tst_strstatus.c fails on Alpine
  2021-07-12 17:02 [LTP] tst_strstatus.c fails on Alpine Petr Vorel
@ 2021-07-13  6:08 ` Jan Stancek
  2021-07-13  7:50   ` Alexey Kodanev
  2021-07-13  8:57   ` Petr Vorel
  0 siblings, 2 replies; 8+ messages in thread
From: Jan Stancek @ 2021-07-13  6:08 UTC (permalink / raw)
  To: ltp

On Mon, Jul 12, 2021 at 7:02 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi all,
>
> I see failures of lib/newlib_tests/tst_strstatus on Alpine:
>
> tst_strstatus.c:31: TPASS: exited with 1
> tst_strstatus.c:31: TPASS: killed by SIGHUP
> tst_strstatus.c:31: TPASS: is stopped
> tst_strstatus.c:31: TPASS: is resumed
> tst_strstatus.c:29: TFAIL: killed by ??? != invalid status 0xff
>
> Any idea what could be wrong?
>

I'd start with definition of WIFSIGNALED on that system.

printf("%d\n", WIFSIGNALED(0xff));
should give you 0, but it does appear to return 1 in output above.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210713/af0b4658/attachment.htm>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [LTP] tst_strstatus.c fails on Alpine
  2021-07-13  6:08 ` Jan Stancek
@ 2021-07-13  7:50   ` Alexey Kodanev
  2021-07-13  9:26     ` Petr Vorel
  2021-07-13  8:57   ` Petr Vorel
  1 sibling, 1 reply; 8+ messages in thread
From: Alexey Kodanev @ 2021-07-13  7:50 UTC (permalink / raw)
  To: ltp

On 13.07.2021 09:08, Jan Stancek wrote:
> 
> 
> On Mon, Jul 12, 2021 at 7:02 PM Petr Vorel <pvorel@suse.cz <mailto:pvorel@suse.cz>> wrote:
> 
>     Hi all,
> 
>     I see failures of lib/newlib_tests/tst_strstatus on Alpine:
> 
>     tst_strstatus.c:31: TPASS: exited with 1
>     tst_strstatus.c:31: TPASS: killed by SIGHUP
>     tst_strstatus.c:31: TPASS: is stopped
>     tst_strstatus.c:31: TPASS: is resumed
>     tst_strstatus.c:29: TFAIL: killed by ??? != invalid status 0xff
> 
>     Any idea what could be wrong?
> 
> 
> I'd start with definition of WIFSIGNALED on that system.
> 
> printf("%d\n", WIFSIGNALED(0xff));
> should give you 0, but it does appear to return 1 in output above.
> 

musl defines it as:
#define WIFSIGNALED(s) (((s)&0xffff)-1U < 0xffu)
so passing 0xff trigger this.

This difference from the glibc have appeared since the commit:
41c632824c08 ("fix definitions of WIFSTOPPED and WIFSIGNALED to support up to signal 127")

May be changed 0xff to 0x1ff for invalid status...

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [LTP] tst_strstatus.c fails on Alpine
  2021-07-13  6:08 ` Jan Stancek
  2021-07-13  7:50   ` Alexey Kodanev
@ 2021-07-13  8:57   ` Petr Vorel
  2021-07-13  9:25     ` Cyril Hrubis
  1 sibling, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2021-07-13  8:57 UTC (permalink / raw)
  To: ltp

Hi Jan,

> On Mon, Jul 12, 2021 at 7:02 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi all,

> > I see failures of lib/newlib_tests/tst_strstatus on Alpine:

> > tst_strstatus.c:31: TPASS: exited with 1
> > tst_strstatus.c:31: TPASS: killed by SIGHUP
> > tst_strstatus.c:31: TPASS: is stopped
> > tst_strstatus.c:31: TPASS: is resumed
> > tst_strstatus.c:29: TFAIL: killed by ??? != invalid status 0xff

> > Any idea what could be wrong?


> I'd start with definition of WIFSIGNALED on that system.

> printf("%d\n", WIFSIGNALED(0xff));
> should give you 0, but it does appear to return 1 in output above.

Thanks for a hint. Indeed WIFSIGNALED(0xff) returns 1, thus tst_strstatus()
returns signaled(status).

musl defines WIFSIGNALED() as:

#define WIFSIGNALED(s) (((s)&0xffff)-1U < 0xffu)

which returns 1.

Glibc defines __WIFSIGNALED() as:

#define __WIFSIGNALED(status) \
  (((signed char) (((status) & 0x7f) + 1) >> 1) > 0)

which returns 0.

I wonder if it's a musl bug which we should report or {0x100, "invalid status
0xff"} test case is glibc specific and we should guard it with #ifdef __GLIBC__.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [LTP] tst_strstatus.c fails on Alpine
  2021-07-13  8:57   ` Petr Vorel
@ 2021-07-13  9:25     ` Cyril Hrubis
  2021-07-13 10:24       ` Petr Vorel
  0 siblings, 1 reply; 8+ messages in thread
From: Cyril Hrubis @ 2021-07-13  9:25 UTC (permalink / raw)
  To: ltp

Hi!
> Thanks for a hint. Indeed WIFSIGNALED(0xff) returns 1, thus tst_strstatus()
> returns signaled(status).
> 
> musl defines WIFSIGNALED() as:
> 
> #define WIFSIGNALED(s) (((s)&0xffff)-1U < 0xffu)
> 
> which returns 1.
> 
> Glibc defines __WIFSIGNALED() as:
> 
> #define __WIFSIGNALED(status) \
>   (((signed char) (((status) & 0x7f) + 1) >> 1) > 0)
> 
> which returns 0.
> 
> I wonder if it's a musl bug which we should report or {0x100, "invalid status
> 0xff"} test case is glibc specific and we should guard it with #ifdef __GLIBC__.

The process exit values are defined in the kernel ABI so I would say
that there shouldn't be any differencies between how these are handled
inside different libc implementation. That being said the musl version
is incorrect only for invalid values that will probably not happen in
practice. Glibc is simply more defensive in parsing and rejects invalid
conditions.

WIFSIGNALED() is supposed to return 1 only if process was killed by a
signal, which means that the upper byte of the status is ignored and the
lower byte has to look like:

7 6 5 4 3 2 1 0
x . . . . . . .
  ^
^ Termination signal
|
Core dumped flag

Also this value can't be set tio 0x7f since that means "stopped by
signal".

This is exaclty what glibc does since it masks the termination signal
number with 0x7f then adds 1, which would overlfow to 0x80 if the value
was 0x7f initially and end up being negative. The bitshift is there to
erase the +1 in a case we started with 0.

The musl libc returns 1 if the lower byte is non-zero and the upper byte
is zero, which depends on the fact that the upper byte is unused and
filled in zeroes when the process was killed by a signal and non-zero in
all other cases where the lower byte is non-zero. As long as we get only
valid status from wait() this is going to work fine.

To be honest I like the defensive parsing from libc more than the musl
variant but I'm not 100% sure if this is something that should be added
to musl as well.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [LTP] tst_strstatus.c fails on Alpine
  2021-07-13  7:50   ` Alexey Kodanev
@ 2021-07-13  9:26     ` Petr Vorel
  0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2021-07-13  9:26 UTC (permalink / raw)
  To: ltp

Hi Alexey,

> On 13.07.2021 09:08, Jan Stancek wrote:


> > On Mon, Jul 12, 2021 at 7:02 PM Petr Vorel <pvorel@suse.cz <mailto:pvorel@suse.cz>> wrote:

> >     Hi all,

> >     I see failures of lib/newlib_tests/tst_strstatus on Alpine:

> >     tst_strstatus.c:31: TPASS: exited with 1
> >     tst_strstatus.c:31: TPASS: killed by SIGHUP
> >     tst_strstatus.c:31: TPASS: is stopped
> >     tst_strstatus.c:31: TPASS: is resumed
> >     tst_strstatus.c:29: TFAIL: killed by ??? != invalid status 0xff

> >     Any idea what could be wrong?


> > I'd start with definition of WIFSIGNALED on that system.

> > printf("%d\n", WIFSIGNALED(0xff));
> > should give you 0, but it does appear to return 1 in output above.


> musl defines it as:
> #define WIFSIGNALED(s) (((s)&0xffff)-1U < 0xffu)
> so passing 0xff trigger this.

> This difference from the glibc have appeared since the commit:
> 41c632824c08 ("fix definitions of WIFSTOPPED and WIFSIGNALED to support up to signal 127")

> May be changed 0xff to 0x1ff for invalid status...

Thanks, 0x1ff is the right value for both glibc and musl.
I'll send it as part of v4 	of my patchset "Run tests in CI".

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [LTP] tst_strstatus.c fails on Alpine
  2021-07-13  9:25     ` Cyril Hrubis
@ 2021-07-13 10:24       ` Petr Vorel
  2021-07-13 11:49         ` Cyril Hrubis
  0 siblings, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2021-07-13 10:24 UTC (permalink / raw)
  To: ltp

Hi all,

> Hi!
> > Thanks for a hint. Indeed WIFSIGNALED(0xff) returns 1, thus tst_strstatus()
> > returns signaled(status).

> > musl defines WIFSIGNALED() as:

> > #define WIFSIGNALED(s) (((s)&0xffff)-1U < 0xffu)

> > which returns 1.

> > Glibc defines __WIFSIGNALED() as:

> > #define __WIFSIGNALED(status) \
> >   (((signed char) (((status) & 0x7f) + 1) >> 1) > 0)

> > which returns 0.

> > I wonder if it's a musl bug which we should report or {0x100, "invalid status
> > 0xff"} test case is glibc specific and we should guard it with #ifdef __GLIBC__.

> The process exit values are defined in the kernel ABI so I would say
> that there shouldn't be any differencies between how these are handled
> inside different libc implementation. That being said the musl version
> is incorrect only for invalid values that will probably not happen in
> practice. Glibc is simply more defensive in parsing and rejects invalid
> conditions.
Agree.


> WIFSIGNALED() is supposed to return 1 only if process was killed by a
> signal, which means that the upper byte of the status is ignored and the
> lower byte has to look like:

> 7 6 5 4 3 2 1 0
> x . . . . . . .
>   ^
> ^ Termination signal

> Core dumped flag

> Also this value can't be set tio 0x7f since that means "stopped by
> signal".

> This is exaclty what glibc does since it masks the termination signal
> number with 0x7f then adds 1, which would overlfow to 0x80 if the value
> was 0x7f initially and end up being negative. The bitshift is there to
> erase the +1 in a case we started with 0.

> The musl libc returns 1 if the lower byte is non-zero and the upper byte
> is zero, which depends on the fact that the upper byte is unused and
> filled in zeroes when the process was killed by a signal and non-zero in
> all other cases where the lower byte is non-zero. As long as we get only
> valid status from wait() this is going to work fine.

Thanks for a detailed explanation.

> To be honest I like the defensive parsing from libc more than the musl
> variant but I'm not 100% sure if this is something that should be added
> to musl as well.

FYI musl commit 41c63282 ("fix definitions of WIFSTOPPED and WIFSIGNALED
to support up to signal 127") [1] mentions mips bug discussion on linux-mips ML
from 2013 (musl fix is also from that time) [2]:

> I think it's feasible to ask {g,uc}libc to change their defines
> (on MIPS as a minimum), and live with 127 signals.

=> I haven't checked if it was posted at the time. I wonder if anybody cares
enough about mips nowadays to fix this. I also like these guarders, thus I
wonder if musl should have it only for mips (currently it's for all archs).


Kind regards,
Petr

[1] https://git.musl-libc.org/cgit/musl/commit/?id=41c632824c08ac2c9eea43b30d1b3515dd910df6
[2] https://www.linux-mips.org/archives/linux-mips/2013-06/msg00552.html


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [LTP] tst_strstatus.c fails on Alpine
  2021-07-13 10:24       ` Petr Vorel
@ 2021-07-13 11:49         ` Cyril Hrubis
  0 siblings, 0 replies; 8+ messages in thread
From: Cyril Hrubis @ 2021-07-13 11:49 UTC (permalink / raw)
  To: ltp

Hi!
> FYI musl commit 41c63282 ("fix definitions of WIFSTOPPED and WIFSIGNALED
> to support up to signal 127") [1] mentions mips bug discussion on linux-mips ML
> from 2013 (musl fix is also from that time) [2]:
> 
> > I think it's feasible to ask {g,uc}libc to change their defines
> > (on MIPS as a minimum), and live with 127 signals.
> 
> => I haven't checked if it was posted at the time. I wonder if anybody cares
> enough about mips nowadays to fix this. I also like these guarders, thus I
> wonder if musl should have it only for mips (currently it's for all archs).

Hmm, as long as you allow signal 127 you really have to look at the
zeroed unused bits to distinguish between stopped and killed by a
signal. And after that the invalid cobinations end up only to be
non-zero in the upper byte and anything else than 0x7f in the lower
byte without the special value 0xffff.

So in addition to the 0x01ff things like 0x0101 etc. are invalid as
well.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-07-13 11:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-12 17:02 [LTP] tst_strstatus.c fails on Alpine Petr Vorel
2021-07-13  6:08 ` Jan Stancek
2021-07-13  7:50   ` Alexey Kodanev
2021-07-13  9:26     ` Petr Vorel
2021-07-13  8:57   ` Petr Vorel
2021-07-13  9:25     ` Cyril Hrubis
2021-07-13 10:24       ` Petr Vorel
2021-07-13 11:49         ` Cyril Hrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.