[LTP] [PATCH] syscalls/timer_getoverrun01: initialize timer variable

[LTP] [PATCH] syscalls/timer_getoverrun01: initialize timer variable

[Jan Stancek]

Testcase is failing on s390x, with glibc-2.39 and 6.9-rc0 (git commit
a4145ce1e7bc). Userspace defines timer_t as void * (8 bytes), while
__kernel_timer_t is defined as int (4 bytes).  This means that kernel
only populates 4 bytes, and other 4 can remain uninitialized, possibly
containing some non-zero garbage, e.g.:

  timer_create(CLOCK_REALTIME, {sigev_signo=SIGALRM, sigev_notify=SIGEV_SIGNAL},  <unfinished ...>
  <... timer_create resumed>[0]) = 0
  timer_getoverrun(1 <unfinished ...>
  timer_getoverrun resumed>) = -1 EINVAL (Invalid argument)
  timer_delete(1)    = -1 EINVAL (Invalid argument)

Initialize entire "timer" variable to 0.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c b/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
index 5c444857aaa7..897bf65a2480 100644
--- a/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
+++ b/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
@@ -25,6 +25,7 @@ static void run(void)
 	timer_t timer;
 	struct sigevent ev;
 
+	memset(&timer, 0, sizeof(timer_t));
 	ev.sigev_value = (union sigval) 0;
 	ev.sigev_notify = SIGEV_SIGNAL;
 	ev.sigev_signo = SIGALRM;
-- 
2.39.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] syscalls/timer_getoverrun01: use kernel_timer_t type

[Jan Stancek]

Testcase is failing on s390x, with glibc-2.39 and 6.9-rc0 (git commit
a4145ce1e7bc). Userspace defines timer_t as void * (8 bytes), while
__kernel_timer_t is defined as int (4 bytes).  This means that kernel
only populates 4 bytes, and other 4 can remain uninitialized, possibly
containing some non-zero garbage, e.g.:

  timer_create(CLOCK_REALTIME, {sigev_signo=SIGALRM, sigev_notify=SIGEV_SIGNAL},  <unfinished ...>
  <... timer_create resumed>[0]) = 0
  timer_getoverrun(1 <unfinished ...>
  timer_getoverrun resumed>) = -1 EINVAL (Invalid argument)
  timer_delete(1)    = -1 EINVAL (Invalid argument)

Since we are dealing with syscalls directly, use kernel_timer_t.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 .../kernel/syscalls/timer_getoverrun/timer_getoverrun01.c      | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c b/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
index 5c444857aaa7..57c2147bf47f 100644
--- a/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
+++ b/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
@@ -19,10 +19,11 @@
 #include <time.h>
 #include "tst_safe_clocks.h"
 #include "lapi/syscalls.h"
+#include "lapi/common_timers.h"
 
 static void run(void)
 {
-	timer_t timer;
+	kernel_timer_t timer;
 	struct sigevent ev;
 
 	ev.sigev_value = (union sigval) 0;
-- 
2.39.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] syscalls/timer_getoverrun01: use kernel_timer_t type

[Li Wang]

On Fri, Mar 22, 2024 at 2:26 AM Jan Stancek <jstancek@redhat.com> wrote:

> Testcase is failing on s390x, with glibc-2.39 and 6.9-rc0 (git commit
> a4145ce1e7bc). Userspace defines timer_t as void * (8 bytes), while
> __kernel_timer_t is defined as int (4 bytes).  This means that kernel
> only populates 4 bytes, and other 4 can remain uninitialized, possibly
> containing some non-zero garbage, e.g.:
>
>   timer_create(CLOCK_REALTIME, {sigev_signo=SIGALRM,
> sigev_notify=SIGEV_SIGNAL},  <unfinished ...>
>   <... timer_create resumed>[0]) = 0
>   timer_getoverrun(1 <unfinished ...>
>   timer_getoverrun resumed>) = -1 EINVAL (Invalid argument)
>   timer_delete(1)    = -1 EINVAL (Invalid argument)
>
> Since we are dealing with syscalls directly, use kernel_timer_t.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
>

Reviewed-by: Li Wang <liwang@redhat.com>

---
>  .../kernel/syscalls/timer_getoverrun/timer_getoverrun01.c      | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git
> a/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
> b/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
> index 5c444857aaa7..57c2147bf47f 100644
> --- a/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
> +++ b/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
> @@ -19,10 +19,11 @@
>  #include <time.h>
>  #include "tst_safe_clocks.h"
>  #include "lapi/syscalls.h"
> +#include "lapi/common_timers.h"
>
>  static void run(void)
>  {
> -       timer_t timer;
> +       kernel_timer_t timer;
>         struct sigevent ev;
>
>         ev.sigev_value = (union sigval) 0;
> --
> 2.39.3
>
>

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] syscalls/timer_getoverrun01: use kernel_timer_t type

[Petr Vorel]

Hi Jan,

...
> +++ b/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
> @@ -19,10 +19,11 @@
>  #include <time.h>
>  #include "tst_safe_clocks.h"
>  #include "lapi/syscalls.h"
> +#include "lapi/common_timers.h"

>  static void run(void)
>  {
> -	timer_t timer;
> +	kernel_timer_t timer;

Good catch.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

BTW in v1 you used memset(&timer, 0, sizeof(timer_t));
Could have we used with current compilers just timer_t timer = {} instead of
memset()? Or what is the reason we keep using memset() instead of {}?

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] syscalls/timer_getoverrun01: use kernel_timer_t type

[Jan Stancek]

On Fri, Mar 22, 2024 at 6:11 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Jan,
>
> ...
> > +++ b/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
> > @@ -19,10 +19,11 @@
> >  #include <time.h>
> >  #include "tst_safe_clocks.h"
> >  #include "lapi/syscalls.h"
> > +#include "lapi/common_timers.h"
>
> >  static void run(void)
> >  {
> > -     timer_t timer;
> > +     kernel_timer_t timer;
>
> Good catch.
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> BTW in v1 you used memset(&timer, 0, sizeof(timer_t));
> Could have we used with current compilers just timer_t timer = {} instead of
> memset()? Or what is the reason we keep using memset() instead of {}?

I treated it as opaque type. If it was a scalar, plain {} doesn't work with all
compilers. For example this fails to compile for me with gcc 11:
int main() { int i = {}; return i; }

$ gcc  -Wpedantic a.c
a.c: In function ‘main’:
a.c:1:22: warning: ISO C forbids empty initializer braces [-Wpedantic]
    1 | int main() { int i = {}; return i; }
      |                      ^
a.c:1:22: error: empty scalar initializer
a.c:1:22: note: (near initialization for ‘i’)

"{0}" should work - I guess I was just being too careful.

>
> Kind regards,
> Petr
>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] syscalls/timer_getoverrun01: use kernel_timer_t type

[Petr Vorel]

> On Fri, Mar 22, 2024 at 6:11 AM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Jan,

> > ...
> > > +++ b/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
> > > @@ -19,10 +19,11 @@
> > >  #include <time.h>
> > >  #include "tst_safe_clocks.h"
> > >  #include "lapi/syscalls.h"
> > > +#include "lapi/common_timers.h"

> > >  static void run(void)
> > >  {
> > > -     timer_t timer;
> > > +     kernel_timer_t timer;

> > Good catch.
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>

> > BTW in v1 you used memset(&timer, 0, sizeof(timer_t));
> > Could have we used with current compilers just timer_t timer = {} instead of
> > memset()? Or what is the reason we keep using memset() instead of {}?

> I treated it as opaque type. If it was a scalar, plain {} doesn't work with all
> compilers. For example this fails to compile for me with gcc 11:
> int main() { int i = {}; return i; }

> $ gcc  -Wpedantic a.c
> a.c: In function ‘main’:
> a.c:1:22: warning: ISO C forbids empty initializer braces [-Wpedantic]
>     1 | int main() { int i = {}; return i; }
>       |                      ^
> a.c:1:22: error: empty scalar initializer
> a.c:1:22: note: (near initialization for ‘i’)

> "{0}" should work - I guess I was just being too careful.

Thanks for info, Jan. I suppose the problem is also on older
gcc (nothing gcc 11 specific). We compile also on gcc 4.8 and 7.3.

Is it also problem for older clang?

If I understand correctly it should be safe to use {0}, and {} probably waits
for the future.

Kind regards,
Petr

> > Kind regards,
> > Petr



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] syscalls/timer_getoverrun01: use kernel_timer_t type

[Jan Stancek]

On Fri, Mar 22, 2024 at 10:35 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> > On Fri, Mar 22, 2024 at 6:11 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > Hi Jan,
>
> > > ...
> > > > +++ b/testcases/kernel/syscalls/timer_getoverrun/timer_getoverrun01.c
> > > > @@ -19,10 +19,11 @@
> > > >  #include <time.h>
> > > >  #include "tst_safe_clocks.h"
> > > >  #include "lapi/syscalls.h"
> > > > +#include "lapi/common_timers.h"
>
> > > >  static void run(void)
> > > >  {
> > > > -     timer_t timer;
> > > > +     kernel_timer_t timer;
>
> > > Good catch.
> > > Reviewed-by: Petr Vorel <pvorel@suse.cz>

Pushed.

>
> > > BTW in v1 you used memset(&timer, 0, sizeof(timer_t));
> > > Could have we used with current compilers just timer_t timer = {} instead of
> > > memset()? Or what is the reason we keep using memset() instead of {}?
>
> > I treated it as opaque type. If it was a scalar, plain {} doesn't work with all
> > compilers. For example this fails to compile for me with gcc 11:
> > int main() { int i = {}; return i; }
>
> > $ gcc  -Wpedantic a.c
> > a.c: In function ‘main’:
> > a.c:1:22: warning: ISO C forbids empty initializer braces [-Wpedantic]
> >     1 | int main() { int i = {}; return i; }
> >       |                      ^
> > a.c:1:22: error: empty scalar initializer
> > a.c:1:22: note: (near initialization for ‘i’)
>
> > "{0}" should work - I guess I was just being too careful.
>
> Thanks for info, Jan. I suppose the problem is also on older
> gcc (nothing gcc 11 specific). We compile also on gcc 4.8 and 7.3.
>
> Is it also problem for older clang?

It is with clang 16:

# clang a.c
a.c:1:22: error: scalar initializer cannot be empty
int main() { int i = {}; return i; }

>
> If I understand correctly it should be safe to use {0}, and {} probably waits
> for the future.
>
> Kind regards,
> Petr
>
> > > Kind regards,
> > > Petr
>
>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp