| Message ID | 1622713444-21197-1-git-send-email-xuyang2018.jy@fujitsu.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] syscalls/mallinfo2_01: Add a basic test for mallinfo2 when setting 2G size | expand |
Hi Xu, ... > +++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h > @@ -1,5 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > + * Copyright (C) 2020 Free Software Foundation, Inc. > * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved. > * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com> > */ > @@ -28,4 +29,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m) > } > #endif > +#ifdef HAVE_MALLINFO2 > +static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m) > +{ > + tst_res(TINFO, "%s...", msg); nit: I'd remove "..." and add extra space before #define (readability) And also change it on previously added print_mallinfo(). > +#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f) > + P2(arena); > + P2(ordblks); > + P2(smblks); > + P2(hblks); > + P2(hblkhd); > + P2(usmblks); > + P2(fsmblks); > + P2(uordblks); > + P2(fordblks); > + P2(keepcost); > +} > +#endif ... > +++ b/testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c > @@ -0,0 +1,46 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved. > + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> > + */ > + > +/*\ > + * [DESCRIPTION] > + * > + * Basic mallinfo2() test. > + * > + * Test members of struct mallinfo2() whether overflow when setting 2G size. * Test hblkhd member of struct mallinfo2 whether overflow when setting 2G size. > + */ > + > +#include "mallinfo_common.h" > +#include "tst_safe_macros.h" > + > +#ifdef HAVE_MALLINFO2 > + > +void test_mallinfo2(void) > +{ > + struct mallinfo2 info; > + char *buf; > + size_t size = 2UL * 1024UL * 1024UL * 1024UL; > + > + buf = malloc(size); > + if (!buf) { > + tst_res(TCONF, "Current system can not malloc 2G space, skip it"); > + return; > + } Here could be just: if (!buf) tst_brk(TCONF, "Current system can not malloc 2G space, skip it"); > + info = mallinfo2(); > + if (info.hblkhd < size) { > + print_mallinfo2("Test malloc 2G", &info); > + tst_brk(TFAIL, "The member of struct mallinfo2 overflow?"); You don't free buf here. Maybe something like: if (info.hblkhd < size) { print_mallinfo2("Test malloc 2G", &info); tst_res(TFAIL, "hblkhd member of struct mallinfo2 overflow?"); } else { tst_res(TPASS, "hblkhd member of struct mallinfo2 doesn't overflow"); } free(buf); If you're ok with it, no need to repost, I'll change it before merge. Kind regards, Petr
Hi Petr > Hi Xu, > > ... >> +++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h >> @@ -1,5 +1,6 @@ >> // SPDX-License-Identifier: GPL-2.0-or-later >> /* >> + * Copyright (C) 2020 Free Software Foundation, Inc. >> * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved. >> * Author: Yang Xu<xuyang2018.jy@cn.fujitsu.com> >> */ >> @@ -28,4 +29,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m) >> } >> #endif > >> +#ifdef HAVE_MALLINFO2 >> +static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m) >> +{ >> + tst_res(TINFO, "%s...", msg); > nit: I'd remove "..." and add extra space before #define (readability) > And also change it on previously added print_mallinfo(). > >> +#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f) >> + P2(arena); >> + P2(ordblks); >> + P2(smblks); >> + P2(hblks); >> + P2(hblkhd); >> + P2(usmblks); >> + P2(fsmblks); >> + P2(uordblks); >> + P2(fordblks); >> + P2(keepcost); >> +} >> +#endif > > ... >> +++ b/testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c >> @@ -0,0 +1,46 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved. >> + * Author: Yang Xu<xuyang2018.jy@fujitsu.com> >> + */ >> + >> +/*\ >> + * [DESCRIPTION] >> + * >> + * Basic mallinfo2() test. >> + * >> + * Test members of struct mallinfo2() whether overflow when setting 2G size. > * Test hblkhd member of struct mallinfo2 whether overflow when setting 2G size. > >> + */ >> + >> +#include "mallinfo_common.h" >> +#include "tst_safe_macros.h" >> + >> +#ifdef HAVE_MALLINFO2 >> + >> +void test_mallinfo2(void) >> +{ >> + struct mallinfo2 info; >> + char *buf; >> + size_t size = 2UL * 1024UL * 1024UL * 1024UL; >> + >> + buf = malloc(size); >> + if (!buf) { >> + tst_res(TCONF, "Current system can not malloc 2G space, skip it"); >> + return; >> + } > Here could be just: > if (!buf) > tst_brk(TCONF, "Current system can not malloc 2G space, skip it"); > >> + info = mallinfo2(); >> + if (info.hblkhd< size) { >> + print_mallinfo2("Test malloc 2G",&info); >> + tst_brk(TFAIL, "The member of struct mallinfo2 overflow?"); > You don't free buf here. > > Maybe something like: > > if (info.hblkhd< size) { > print_mallinfo2("Test malloc 2G",&info); > tst_res(TFAIL, "hblkhd member of struct mallinfo2 overflow?"); > } else { > tst_res(TPASS, "hblkhd member of struct mallinfo2 doesn't overflow"); > } > > free(buf); > > If you're ok with it, no need to repost, I'll change it before merge. I am ok with these changes. Thanks for your review. Best Regards Yang Xu > > Kind regards, > Petr
Hi Xu, > +++ b/testcases/kernel/syscalls/mallinfo2/Makefile ... > + > +top_srcdir ?= ../../../.. > + > +include $(top_srcdir)/include/mk/testcases.mk > + > +CFLAGS += -I../mallinfo Also this needs to be CFLAGS += -I$(abs_srcdir)/../mallinfo otherwise out of tree builds fail: https://github.com/pevik/ltp/actions/runs/941820122 BTW Cyril also suggested recently to drop out of tree support, because build system dependencies are broken and fixing it would be much easier when supporting only in tree build. Kind regards, Petr
Hi Petr > Hi Xu, > >> +++ b/testcases/kernel/syscalls/mallinfo2/Makefile > ... >> + >> +top_srcdir ?= ../../../.. >> + >> +include $(top_srcdir)/include/mk/testcases.mk >> + >> +CFLAGS += -I../mallinfo > Also this needs to be > CFLAGS += -I$(abs_srcdir)/../mallinfo > You are right. It will fail when out of tree. > otherwise out of tree builds fail: > https://github.com/pevik/ltp/actions/runs/941820122 > > BTW Cyril also suggested recently to drop out of tree support, because build > system dependencies are broken and fixing it would be much easier when > supporting only in tree build. On my test envrionment, I never use out of tree build. So I am fine with removing it. > > Kind regards, > Petr
Hi Xu, Merged with all my suggested changes + slightly adjusted docparse description and commit message. Thanks a lot! Kind regards, Petr
Hi, On 6/16/2021 9:03 AM, Petr Vorel wrote: > > BTW Cyril also suggested recently to drop out of tree support, because build > system dependencies are broken and fixing it would be much easier when > supporting only in tree build. Really? I got the feeling out-of-tree builds are becoming the new normal and even complex pieces of software like the kernel, glibc and gcc are able to be built out-of-tree or even enforce it. Maybe it is time to abandon autoconf/automake and switch to a better build configuration systems like cmake instead of abandoning out-of-tree builds. Jörg
Hi! > > BTW Cyril also suggested recently to drop out of tree support, because build > > system dependencies are broken and fixing it would be much easier when > > supporting only in tree build. > Really? I got the feeling out-of-tree builds are becoming the new normal > and even complex pieces of software like the kernel, glibc and gcc are > able to be built out-of-tree or even enforce it. The question here really if support of out-of-tree build is worth the trouble. The problem is that the complexity it adds to the build system is quite high. The way it's implemented in LTP the build system has to work with absolute paths and you have to make sure you have the right path to a file all the time and people are often confused by this. Also it crashes and burns with cryptic error messages if you happen to have characters interpreted by make in a directory name anywhere from the LTP dir to the filesystems root (try adding : to a directory name for instance). We used to have out-of-tree build broken for years, before we set up a CI, and nobody complained. So considering all of this it may be for the better to just get rid of it. I'm not against out-of-tree build in principle but given that the way it's implemented in LTP is subtly broken by design and that there does not seem to be real users it does not make that much sense to keep it maintained and spend any effort to keep it afloat. > Maybe it is time to abandon autoconf/automake and switch to a better > build configuration systems like cmake instead of abandoning out-of-tree > builds. I'm not againts this, the main problem is that this will take a significant effort my guess is at least month to implement and and even more to iron out all the corner cases. Also the choosen build systems has to be generally available even on 10 years old system, but I guess that cmake should classify easily here.
diff --git a/configure.ac b/configure.ac index 136d82d09..eb675b367 100644 --- a/configure.ac +++ b/configure.ac @@ -102,6 +102,7 @@ AC_CHECK_FUNCS_ONCE([ \ io_uring_enter \ kcmp \ mallinfo \ + mallinfo2 \ mallopt \ mkdirat \ mknodat \ diff --git a/runtest/syscalls b/runtest/syscalls index 9df181b76..11bb58a83 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -692,6 +692,8 @@ lstat02_64 lstat02_64 mallinfo02 mallinfo02 +mallinfo2_01 mallinfo2_01 + mallopt01 mallopt01 mbind01 mbind01 diff --git a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h index a00cc7a64..d64a9344b 100644 --- a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h +++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-or-later /* + * Copyright (C) 2020 Free Software Foundation, Inc. * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved. * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com> */ @@ -28,4 +29,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m) } #endif +#ifdef HAVE_MALLINFO2 +static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m) +{ + tst_res(TINFO, "%s...", msg); +#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f) + P2(arena); + P2(ordblks); + P2(smblks); + P2(hblks); + P2(hblkhd); + P2(usmblks); + P2(fsmblks); + P2(uordblks); + P2(fordblks); + P2(keepcost); +} +#endif + #endif diff --git a/testcases/kernel/syscalls/mallinfo2/.gitignore b/testcases/kernel/syscalls/mallinfo2/.gitignore new file mode 100644 index 000000000..349222c14 --- /dev/null +++ b/testcases/kernel/syscalls/mallinfo2/.gitignore @@ -0,0 +1 @@ +/mallinfo2_01 diff --git a/testcases/kernel/syscalls/mallinfo2/Makefile b/testcases/kernel/syscalls/mallinfo2/Makefile new file mode 100644 index 000000000..b3c7d9552 --- /dev/null +++ b/testcases/kernel/syscalls/mallinfo2/Makefile @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (c) International Business Machines Corp., 2001 +# Copyright (c) 2021 FUJITSU LIMITED. All rights reserved. + +top_srcdir ?= ../../../.. + +include $(top_srcdir)/include/mk/testcases.mk + +CFLAGS += -I../mallinfo + +include $(top_srcdir)/include/mk/generic_leaf_target.mk diff --git a/testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c b/testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c new file mode 100644 index 000000000..9811f95c0 --- /dev/null +++ b/testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved. + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> + */ + +/*\ + * [DESCRIPTION] + * + * Basic mallinfo2() test. + * + * Test members of struct mallinfo2() whether overflow when setting 2G size. + */ + +#include "mallinfo_common.h" +#include "tst_safe_macros.h" + +#ifdef HAVE_MALLINFO2 + +void test_mallinfo2(void) +{ + struct mallinfo2 info; + char *buf; + size_t size = 2UL * 1024UL * 1024UL * 1024UL; + + buf = malloc(size); + if (!buf) { + tst_res(TCONF, "Current system can not malloc 2G space, skip it"); + return; + } + info = mallinfo2(); + if (info.hblkhd < size) { + print_mallinfo2("Test malloc 2G", &info); + tst_brk(TFAIL, "The member of struct mallinfo2 overflow?"); + } + tst_res(TPASS, "The member of struct mallinfo2 doesn't overflow"); + free(buf); +} + +static struct tst_test test = { + .test_all = test_mallinfo2, +}; + +#else +TST_TEST_TCONF("system doesn't implement non-POSIX mallinfo2()"); +#endif
Since these members of mallinfo struct use int data type, it will overflow when allocating 2G size. mallinfo() is deprecated and we should use mallinfo2() in the future. So we test mallinfo2 whether works well. Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> --- configure.ac | 1 + runtest/syscalls | 2 + .../syscalls/mallinfo/mallinfo_common.h | 19 ++++++++ .../kernel/syscalls/mallinfo2/.gitignore | 1 + testcases/kernel/syscalls/mallinfo2/Makefile | 11 +++++ .../kernel/syscalls/mallinfo2/mallinfo2_01.c | 46 +++++++++++++++++++ 6 files changed, 80 insertions(+) create mode 100644 testcases/kernel/syscalls/mallinfo2/.gitignore create mode 100644 testcases/kernel/syscalls/mallinfo2/Makefile create mode 100644 testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c