On 1/21/22 15:34, Jeffrey Walton wrote: > On Fri, Jan 21, 2022 at 3:19 AM Dumitru Ceara <[email protected]> wrote: >> >> On 1/21/22 02:12, Jeffrey Walton wrote: >> >>> I am testing Open vSwitch 2.16.2. Undefined Behavior sanitizer is >>> producing some findings. >>> >>> My question is, is the undefined behavior something the project would >>> be interested in fixing? >>> >> >> I think so. >> >>> If so, I can send over the findings and a proposed patch. If not, I >>> can carry the patch privately. >>> >> >> I had posted a patch series to take care of the issues currently >> reported by UB Sanitizer: >> >> https://patchwork.ozlabs.org/project/openvswitch/list/?series=277900&state=* >> >> It will need a new revision and Adrian (in cc) is also working on some >> more patches to deal with other UB (not flagged by UB Sanitizer). Would >> the patch you were planning to propose overlap with this work? >> >> In any case, I think it would be great if we could work together >> (reviews and patches) to get tests passing with UB Sanitizer and ideally >> running these kinds of checks automatically, in CI. E.g., with the >> series above applied all unit tests are also run with UB Sanitizer enabled: >> >> https://github.com/dceara/ovs/runs/4593681037?check_suite_focus=true > > Awesome. > > In case anyone else is interested... > > export CFLAGS="-fsanitize=undefined > -fno-sanitize=integer-divide-by-zero > -fno-sanitize=float-divide-by-zero -fno-sanitize-recover=all" > export CXXFLAGS="-fsanitize=undefined > -fno-sanitize=integer-divide-by-zero > -fno-sanitize=float-divide-by-zero -fno-sanitize-recover=all" > export LDFLAGS="-fsanitize=undefined > -fno-sanitize=integer-divide-by-zero > -fno-sanitize=float-divide-by-zero -fno-sanitize-recover=all" > > We don't need the divide-by-0 findings. They are usually false > positives. IEEE-754 leaves it up to the implementation to trap. > > Then, configure && make && make check. Once the self tests run (or hang): > $ grep -IR 'runtime error' ./* 2>/dev/null > ./tests/testsuite.dir/0245/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7fff61547e0c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0262/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7fff3954c21c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0408/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7fff3e1cf8ac for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0397/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffee8037d0c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0410/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffd62e8585c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0211/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffc5e3ecb0c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0051/testsuite.log:+tests/test-hash.c:59:40: > runtime error: shift exponent 64 is too large for 64-bit type 'long > unsigned int' > ./tests/testsuite.dir/0190/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffff13fe65c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0302/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffc41b735bc for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0331/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffd6c61c2bc for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0381/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7fff9f4a0a3c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0358/stderr:lib/hash.h:219:17: runtime error: > load of misaligned address 0x7ffca418c60c for type 'const uint64_t', > which requires 8 byte alignment > ./tests/testsuite.dir/0358/testsuite.log:lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffca418c60c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0247/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7fff27ad680c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0124/testsuite.log:+lib/classifier.c:1763:5: > runtime error: applying non-zero offset 18446744073709551592 to null > pointer > ./tests/testsuite.dir/0285/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffeb2264bec for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0384/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7fff69754d9c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0417/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffc27b1a5bc for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0054/testsuite.log:+tests/test-cmap.c:106:9: > runtime error: applying non-zero offset 18446744073709551608 to null > pointer > ./tests/testsuite.dir/0406/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffeb01ed1bc for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0366/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7fffc6c1783c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0226/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffff0cdb64c for type 'const > uint64_t', which requires 8 byte alignment > ./tests/testsuite.dir/0053/testsuite.log:+tests/test-hindex.c:60:5: > runtime error: applying non-zero offset 18446744073709551608 to null > pointer > ./tests/testsuite.dir/0426/stderr:lib/nx-match.c:2311:5: runtime > error: applying non-zero offset 18446744073709551600 to null pointer > ./tests/testsuite.dir/0426/testsuite.log:lib/nx-match.c:2311:5: > runtime error: applying non-zero offset 18446744073709551600 to null > pointer > ./tests/testsuite.dir/0325/testsuite.log:+lib/hash.h:219:17: runtime > error: load of misaligned address 0x7ffe609f708c for type 'const > uint64_t', which requires 8 byte alignment > ... > > I see the findings in lib/hash.h:219 because I also compile with > -march=native, and the machine has AVX2,AVX,SSE42,SSE41,... > lib/hash.h:219 is due to the uint64_t* cast on a 32-bit aligned > buffer.
Interesting, I see this now too after switching to -march=native. Do you already have a fix for this in mind? I don't see a way of fixing it without changing the SSE42 version of hash_words_inline() to either: a. implement a different hash function or b. switch to hashing one u32 at a time if the initial buffer is not 64-but aligned. I'm suspecting both these approaches will affect performance. Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
