On 1/24/22 13:36, Dumitru Ceara wrote: > On 1/21/22 15:34, Jeffrey Walton wrote: >> On Fri, Jan 21, 2022 at 3:19 AM Dumitru Ceara <dce...@redhat.com> 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. >
In any case, I sent a v3 of my series, fixing most of the issues that are reported when compiling without -march=native/-msse42. https://patchwork.ozlabs.org/project/openvswitch/list/?series=282541&state=* https://mail.openvswitch.org/pipermail/ovs-dev/2022-January/390944.html Regards, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
