Fix annoying warnings while building tests #976

nyh · 2018-05-28T11:21:50Z

We've been careful to clear any compilation warnings during the main OSv build, either by fixing the code or adding a compilation parameter to ignore specific false alarms. However, we've been less diligent in the tests ("scripts/build image=tests", or "make check") and those produce quite a number of silly warning messages during the build. With gcc 8.1.1, I see warnings while building the following tests:

tst-pthread
tst-mmap
misc-fsx
tst-truncate
misc-free-perf
tst-zfs-mount
tst-run
tst-calloc
tst-crypt
tst-vfs
misc-fs-stress
tst-stat
tst-rename
tst-fallocate
tst-fslink

Most of these warnings are silly and can be easily fixed

The text was updated successfully, but these errors were encountered:

nyh · 2018-10-31T13:34:59Z

The list of warnings during test compilation continues to grow in gcc 8.2 :-(

geraldo-netto · 2018-10-31T14:01:57Z

Hi @nyh / @wkozaczuk ,

How are you doing?

My Friends, doing a quick run, I've got this:
In file included from /home/netto/Desktop/osv/osv-tomcat9/tests/tst-elf-permissions.cc:8:0:

/home/netto/Desktop/osv/osv-tomcat9/tests/tst-mmap.hh: In function ‘bool try_write(int ()())’:
/home/netto/Desktop/osv/osv-tomcat9/tests/tst-mmap.hh:76:37: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
char byte = (volatile char)&func;
^~~~
/home/netto/Desktop/osv/osv-tomcat9/tests/tst-mmap.hh:77:25: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
(volatile char)&func = byte;
^~~~
In file included from /home/netto/Desktop/osv/osv-tomcat9/tests/tst-mmap.cc:11:0:
/home/netto/Desktop/osv/osv-tomcat9/tests/tst-mmap.hh: In function ‘bool try_write(int ()())’:
/home/netto/Desktop/osv/osv-tomcat9/tests/tst-mmap.hh:76:37: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
char byte = (volatile char)&func;
^~~~
/home/netto/Desktop/osv/osv-tomcat9/tests/tst-mmap.hh:77:25: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
(volatile char)&func = byte;
^~~~

/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc: In function ‘void test_across_core_alloc_and_free(Alloc, Dealloc)’:
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:69:35: warning: ‘new’ of type ‘queue_t {aka ring_spsc<void*, 67108864>}’ with extended alignment 64 [-Waligned-new=]
queue_t* queue1 = new queue_t();
^
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:69:35: note: uses ‘void* operator new(std::size_t)’, which does not have an alignment parameter
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:69:35: note: use ‘-faligned-new’ to enable C++17 over-aligned new support
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:70:35: warning: ‘new’ of type ‘queue_t {aka ring_spsc<void*, 67108864>}’ with extended alignment 64 [-Waligned-new=]
queue_t* queue2 = new queue_t();
^
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:70:35: note: uses ‘void* operator new(std::size_t)’, which does not have an alignment parameter
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:70:35: note: use ‘-faligned-new’ to enable C++17 over-aligned new support
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc: In instantiation of ‘void test_across_core_alloc_and_free(Alloc, Dealloc) [with Alloc = std::_Bind<void* ((int))(long unsigned int)>; Dealloc = void ()(void*)]’:
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:146:66: required from here
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:69:23: warning: ‘new’ of type ‘queue_t {aka ring_spsc<void*, 67108864>}’ with extended alignment 64 [-Waligned-new=]
queue_t* queue1 = new queue_t();
^~~~~~~~~~~~~
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:69:23: note: uses ‘void* operator new(std::size_t)’, which does not have an alignment parameter
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:69:23: note: use ‘-faligned-new’ to enable C++17 over-aligned new support
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:70:23: warning: ‘new’ of type ‘queue_t {aka ring_spsc<void*, 67108864>}’ with extended alignment 64 [-Waligned-new=]
queue_t* queue2 = new queue_t();
^~~~~~~~~~~~~
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:70:23: note: uses ‘void* operator new(std::size_t)’, which does not have an alignment parameter
/home/netto/Desktop/osv/osv-tomcat9/tests/misc-free-perf.cc:70:23: note: use ‘-faligned-new’ to enable C++17 over-aligned new support

/home/netto/Desktop/osv/osv-tomcat9/build/release.x64/tests/tst-truncate.o: In function main': /home/netto/Desktop/osv/osv-tomcat9/tests/tst-truncate.c:10: warning: the use of tmpnam' is dangerous, better use `mkstemp'

/home/netto/Desktop/osv/osv-tomcat9/build/release.x64/tests/tst-zfs-mount.o: In function check_zfs_refcnt_behavior()': /home/netto/Desktop/osv/osv-tomcat9/tests/tst-zfs-mount.cc:54: warning: the use of mktemp' is dangerous, better use mkstemp' or mkdtemp'

/home/netto/Desktop/osv/osv-tomcat9/build/release.x64/tests/tst-stat.o: In function `test_mkdir_fails_if_file_exists::test_method()':

/home/netto/Desktop/osv/osv-tomcat9/tests/tst-fs.hh:25: warning: the use of tmpnam' is dangerous, better use mkstemp'

/home/netto/Desktop/osv/osv-tomcat9/build/release.x64/tests/misc-fs-stress.o: In function `main':

/home/netto/Desktop/osv/osv-tomcat9/tests/misc-fs-stress.cc:54: warning: the use of mktemp' is dangerous, better use mkstemp' or `mkdtemp'

/home/netto/Desktop/osv/osv-tomcat9/build/release.x64/tests/tst-vfs.o: In function `test_path_lookup::test_method()':

/home/netto/Desktop/osv/osv-tomcat9/tests/tst-fs.hh:25: warning: the use of tmpnam' is dangerous, better use mkstemp'

/home/netto/Desktop/osv/osv-tomcat9/build/release.x64/tests/tst-fallocate.o: In function main': /home/netto/Desktop/osv/osv-tomcat9/tests/tst-fallocate.cc:52: warning: the use of mktemp' is dangerous, better use mkstemp' or mkdtemp'

/home/netto/Desktop/osv/osv-tomcat9/build/release.x64/tests/tst-fs-link.o: In function check_vnode_duplicity': /home/netto/Desktop/osv/osv-tomcat9/tests/tst-fs-link.cc:39: warning: the use of mktemp' is dangerous, better use mkstemp' or mkdtemp'

/home/netto/Desktop/osv/osv-tomcat9/build/release.x64/tests/tst-rename.o: In function `TempDir::TempDir()':

/home/netto/Desktop/osv/osv-tomcat9/tests/tst-fs.hh:25: warning: the use of tmpnam' is dangerous, better use mkstemp'

I hope to find some time to help with those small things, let's keep in touch! :)

Kind Regards,
Geraldo Netto

geraldo-netto · 2018-10-31T14:21:55Z

Guys,

How can I run a specific test?
eg: tst-pthread

Kind Regards,
Geraldo Netto

wkozaczuk · 2018-10-31T14:23:57Z

./scripts/test.py —name tst-pthread.so First you have to build image with all tests.

…

Sent from my iPhone

On Oct 31, 2018, at 10:22, Geraldo Netto ***@***.***> wrote: Guys, How can I run a specific test? eg: tst-pthread Kind Regards, Geraldo Netto — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

geraldo-netto · 2018-10-31T14:33:59Z

Thanks Waldek :)

I think I fixed the first pthread issue I have found, hope to commit in a few minutes
I'm also creating a page on wiki to document it
It's a bare simple page but at least, it's a start :)

See You! :)

geraldo-netto · 2018-10-31T14:45:31Z

Guys,

I have created this page to document how to test
As I said, I still need to polish it, but it's a start
https://github.com/cloudius-systems/osv/wiki/Testing-POSIX-APIs-on-OSv

wkozaczuk · 2018-10-31T14:54:21Z

Geraldo, Thanks for taking on it. Could you please prepare series of fairly small patches targeting specific test or type of warning issue across all affected tests? So it is easier to review. Waldek

…

On Wed, Oct 31, 2018 at 10:46 AM Geraldo Netto ***@***.***> wrote: Guys, I have created this page to document how to test As I said, I still need to polish it, but it's a start https://github.com/cloudius-systems/osv/wiki/Testing-POSIX-APIs-on-OSv — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#976 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFDSIbQJL_zGbaBeNjsCwB8YHC8FoqnLks5uqbfOgaJpZM4UQC6o> .

When building tests, we get this warning: tst-mmap.hh:76:19: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] 76 | char byte = **(volatile char**)&func; | ^~~~~~~~~~~~~~~~~~~~~~ I think this code was actually wrong... func is a function pointer, not a function, so its name already points to the function's code - there is no reason to take &func. I'm not even sure how this works. After this patch, the warning is gone, and the relevant test (tst-elf-permissions.so) still passes. Refs #976 Signed-off-by: Nadav Har'El <[email protected]> Message-Id: <[email protected]>

The compiler correctly warns that we can't fill 4097 bytes of an array size 4096 (=PATH_MAX). I don't know why this code had 4097 in the first place. The test still passes with this fix, and the warning is gone. Refs #976. Signed-off-by: Nadav Har'El <[email protected]> Message-Id: <[email protected]>

It is generally a bad idea to catch exceptions by value, because it forces an unnecessary copy of the caught value. It's even worse when the type is polymorphic - in that case the copy will only be a slice of the original type. This is why recent compilers started to warn when code catches a polymorphic type by value. In the specific case fixed here, the warning is spurious, because the caught exception isn't even used. But it's trivial to fix it, and get rid of one more warning during test compilation. Refs #976 Signed-off-by: Nadav Har'El <[email protected]> Message-Id: <[email protected]>

This should be the last patch addressing compiler warnings in unit tests as reported by the issue #976. Refs #976 Signed-off-by: Waldemar Kozaczuk <[email protected]> Message-Id: <[email protected]>

nyh · 2019-12-09T10:13:55Z

The last of these compilation warnings on test build were fixed by several patches by @wkozaczuk so finally closing this issue.

nyh closed this as completed Dec 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix annoying warnings while building tests #976

Fix annoying warnings while building tests #976

nyh commented May 28, 2018

nyh commented Oct 31, 2018

geraldo-netto commented Oct 31, 2018 •

edited

Loading

geraldo-netto commented Oct 31, 2018

wkozaczuk commented Oct 31, 2018 via email

geraldo-netto commented Oct 31, 2018

geraldo-netto commented Oct 31, 2018

wkozaczuk commented Oct 31, 2018 via email

nyh commented Dec 9, 2019

Fix annoying warnings while building tests #976

Fix annoying warnings while building tests #976

Comments

nyh commented May 28, 2018

nyh commented Oct 31, 2018

geraldo-netto commented Oct 31, 2018 • edited Loading

geraldo-netto commented Oct 31, 2018

wkozaczuk commented Oct 31, 2018 via email

geraldo-netto commented Oct 31, 2018

geraldo-netto commented Oct 31, 2018

wkozaczuk commented Oct 31, 2018 via email

nyh commented Dec 9, 2019

geraldo-netto commented Oct 31, 2018 •

edited

Loading