NFD » ndn-cxx

https://pzemtsov.github.io/2016/11/06/bug-story-alignment-on-x86.html

Build with '--with-sanitizer=undefined --with-tests' and run the unit tests.

There are linker errors. Ubuntu 16.04 32-bit.

$ clang++ -v
clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)
Target: i686-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Found candidate GCC installation: /usr/bin/../lib/gcc/i686-linux-gnu/5.4.0
Found candidate GCC installation: /usr/bin/../lib/gcc/i686-linux-gnu/6.0.0
Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/5.4.0
Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/6.0.0
Selected GCC installation: /usr/bin/../lib/gcc/i686-linux-gnu/5.4.0
Candidate multilib: .;@m32
Selected multilib: .;@m32

static library, gold¶

CXX=clang++ ./waf configure --enable-static --disable-shared --with-tests --debug --without-pch --with-sanitizer=undefined

[345/358] Linking build/tests/integrated/face
/usr/bin/../lib/gcc/i686-linux-gnu/5.4.0/../../../../include/c++/5.4.0/chrono:176: error: undefined reference to '__mulodi4'
/usr/include/boost/chrono/duration.hpp:277: error: undefined reference to '__mulodi4'
/usr/include/boost/chrono/duration.hpp:277: error: undefined reference to '__mulodi4'
/usr/include/boost/chrono/duration.hpp:277: error: undefined reference to '__mulodi4'
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Waf: Leaving directory `/home/ubuntu/clang-ndn-cxx/build'
Build failed
 -> task in 'face' failed (exit status 1): 
    {task 3068172300L: cxxprogram face.cpp.1.o,identity-management-fixture.cpp.2.o -> face}
['clang++', '-fuse-ld=gold', '-Wl,-O1', '-fsanitize=undefined', '-pthread', 'tests/integrated/face.cpp.1.o', 'tests/identity-management-fixture.cpp.2.o', '-o', '/home/ubuntu/clang-ndn-cxx/build/tests/integrated/face', '-Wl,-Bstatic', '-L.', '-L/usr/lib/i386-linux-gnu', '-lndn-cxx', '-Wl,-Bdynamic', '-L/usr/lib/i386-linux-gnu', '-L/usr/lib', '-L/usr/lib', '-lboost_system', '-lboost_filesystem', '-lboost_date_time', '-lboost_iostreams', '-lboost_regex', '-lboost_program_options', '-lboost_chrono', '-lboost_thread', '-lboost_log', '-lboost_log_setup', '-lboost_unit_test_framework', '-lcryptopp', '-lssl', '-lcrypto', '-lsqlite3', '-lrt', '-lpthread']

static library, ld¶

revert ndn-cxx:commit:37271c5b48d60b05dd4f5aa5c719cf5e3e46bf7a

CXX=clang++ ./waf configure --enable-static --disable-shared --with-tests --debug --without-pch --with-sanitizer=undefined

[354/358] Linking build/tests/integrated/scheduler-benchmark
tests/integrated/scheduler-benchmark.cpp.3.o: In function `boost::chrono::detail::duration_cast_aux<boost::chrono::duration<long long, boost::ratio<1ll, 1ll> >, boost::chrono::duration<long long, boost::ratio<1ll, 1000000000ll> >, boost::ratio<1000000000ll, 1ll>, false, true>::operator()(boost::chrono::duration<long long, boost::ratio<1ll, 1ll> > const&) const':
/usr/include/boost/chrono/duration.hpp:277: undefined reference to `__mulodi4'
tests/integrated/scheduler-benchmark.cpp.3.o: In function `boost::chrono::detail::duration_cast_aux<boost::chrono::duration<long long, boost::ratio<1ll, 1000ll> >, boost::chrono::duration<long long, boost::ratio<1ll, 1000000000ll> >, boost::ratio<1000000ll, 1ll>, false, true>::operator()(boost::chrono::duration<long long, boost::ratio<1ll, 1000ll> > const&) const':
/usr/include/boost/chrono/duration.hpp:277: undefined reference to `__mulodi4'
tests/identity-management-fixture.cpp.2.o: In function `boost::chrono::detail::duration_cast_aux<boost::chrono::duration<int, boost::ratio<86400ll, 1ll> >, boost::chrono::duration<long long, boost::ratio<1ll, 1000000000ll> >, boost::ratio<86400000000000ll, 1ll>, false, true>::operator()(boost::chrono::duration<int, boost::ratio<86400ll, 1ll> > const&) const':
/usr/include/boost/chrono/duration.hpp:277: undefined reference to `__mulodi4'
./libndn-cxx.a(name-component.cpp.2.o): In function `boost::chrono::detail::duration_cast_aux<boost::chrono::duration<long long, boost::ratio<1ll, 1000000ll> >, boost::chrono::duration<long long, boost::ratio<1ll, 1000000000ll> >, boost::ratio<1000ll, 1ll>, false, true>::operator()(boost::chrono::duration<long long, boost::ratio<1ll, 1000000ll> > const&) const':
/usr/include/boost/chrono/duration.hpp:277: undefined reference to `__mulodi4'
./libndn-cxx.a(certificate.cpp.2.o): In function `boost::chrono::detail::duration_cast_aux<boost::chrono::duration<long long, boost::ratio<1ll, 1ll> >, boost::chrono::duration<long long, boost::ratio<1ll, 1000ll> >, boost::ratio<1000ll, 1ll>, false, true>::operator()(boost::chrono::duration<long long, boost::ratio<1ll, 1ll> > const&) const':
/usr/include/boost/chrono/duration.hpp:277: undefined reference to `__mulodi4'
./libndn-cxx.a(scheduler.cpp.2.o):/usr/include/boost/date_time/time_duration.hpp:111: more undefined references to `__mulodi4' follow
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Waf: Leaving directory `/home/ubuntu/clang-ndn-cxx/build'
Build failed
 -> task in 'scheduler-benchmark' failed (exit status 1): 
    {task 3068178860L: cxxprogram scheduler-benchmark.cpp.3.o,identity-management-fixture.cpp.2.o -> scheduler-benchmark}
['clang++', '-fsanitize=undefined', '-pthread', 'tests/integrated/scheduler-benchmark.cpp.3.o', 'tests/identity-management-fixture.cpp.2.o', '-o', '/home/ubuntu/clang-ndn-cxx/build/tests/integrated/scheduler-benchmark', '-Wl,-Bstatic', '-L.', '-L/usr/lib/i386-linux-gnu', '-lndn-cxx', '-Wl,-Bdynamic', '-L/usr/lib/i386-linux-gnu', '-L/usr/lib', '-L/usr/lib', '-lboost_system', '-lboost_filesystem', '-lboost_date_time', '-lboost_iostreams', '-lboost_regex', '-lboost_program_options', '-lboost_chrono', '-lboost_thread', '-lboost_log', '-lboost_log_setup', '-lboost_unit_test_framework', '-lcryptopp', '-lssl', '-lcrypto', '-lsqlite3', '-lrt', '-lpthread']

shared library, gold¶

CXX=clang++ ./waf configure --with-tests --debug --without-pch --with-sanitizer=undefined

[354/359] Linking build/bin/ndnsec
/usr/include/boost/date_time/time_duration.hpp:282: error: undefined reference to '__mulodi4'
/usr/include/boost/date_time/time_system_counted.hpp:42: error: undefined reference to '__mulodi4'
/usr/include/boost/date_time/time_system_counted.hpp:92: error: undefined reference to '__mulodi4'
/usr/include/boost/date_time/time_duration.hpp:111: error: undefined reference to '__mulodi4'
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Waf: Leaving directory `/home/ubuntu/clang-ndn-cxx/build'
Build failed
 -> task in '../bin/ndnsec' failed (exit status 1): 
    {task 3067674540L: cxxprogram cert-dump.cpp.1.o,cert-gen.cpp.1.o,cert-install.cpp.1.o,delete.cpp.1.o,export.cpp.1.o,get-default.cpp.1.o,import.cpp.1.o,key-gen.cpp.1.o,list.cpp.1.o,main.cpp.1.o,set-default.cpp.1.o,sign-req.cpp.1.o,unlock-tpm.cpp.1.o,util.cpp.1.o -> ndnsec}
['clang++', '-fuse-ld=gold', '-Wl,-O1', '-fsanitize=undefined', '-pthread', 'tools/ndnsec/cert-dump.cpp.1.o', 'tools/ndnsec/cert-gen.cpp.1.o', 'tools/ndnsec/cert-install.cpp.1.o', 'tools/ndnsec/delete.cpp.1.o', 'tools/ndnsec/export.cpp.1.o', 'tools/ndnsec/get-default.cpp.1.o', 'tools/ndnsec/import.cpp.1.o', 'tools/ndnsec/key-gen.cpp.1.o', 'tools/ndnsec/list.cpp.1.o', 'tools/ndnsec/main.cpp.1.o', 'tools/ndnsec/set-default.cpp.1.o', 'tools/ndnsec/sign-req.cpp.1.o', 'tools/ndnsec/unlock-tpm.cpp.1.o', 'tools/ndnsec/util.cpp.1.o', '-o', '/home/ubuntu/clang-ndn-cxx/build/bin/ndnsec', '-Wl,-Bstatic', '-L.', '-L/usr/lib/i386-linux-gnu', '-Wl,-Bdynamic', '-L.', '-L/usr/lib/i386-linux-gnu', '-L/usr/lib', '-L/usr/lib', '-lndn-cxx', '-lboost_system', '-lboost_filesystem', '-lboost_date_time', '-lboost_iostreams', '-lboost_regex', '-lboost_program_options', '-lboost_chrono', '-lboost_thread', '-lboost_log', '-lboost_log_setup', '-lboost_unit_test_framework', '-lcryptopp', '-lssl', '-lcrypto', '-lsqlite3', '-lrt', '-lpthread']

Just follow the instructions, why are you adding random extra options? Don't use --debug with the sanitizers, it's useless and just slows everything down.
Also why are you using clang? In any case, clang 4.0.1 works for me.

Junxiao Shi wrote:

• --enable-static --disable-shared selects static instead of shared library, so that I don't have to run sudo ./waf install before running unit tests.

This is a poor reason. Use LD_LIBRARY_PATH instead.

ASan is only available in clang, isn't it?

No. It's available in gcc since 4.8, and UBSan since 4.9.

Junxiao Shi wrote:

Old code needs more than just InputIterator. New code requires InputIterator and nothing more, which is assured by a Boost archetype check.

While we probably do need a version that works on input iterators, in the vast majority of cases decoding operates on memory buffers, which allow random access iterators (or even contiguous). If the decoding can be made more efficient by leveraging stronger properties offered by the iterator, then we should definitely take advantage of that.

Benchmark on Ubuntu 16.04 32-bit indicates new code is 50% slower with 3-octet VarNumber.

Have you tried std::copy and/or memcpy?

in the vast majority of cases decoding operates on memory buffers, which allow random access iterators (or even contiguous).

RandomAccessIterator won't help because it still only allows reading one octet at a time.
Contigous memory allows memcpy. This is to be investigated in #4172.

This is to be investigated in #4172.

I disagree with this way of proceeding.
As you said yourself, the current code already requires more than an input iterator. Specifically, it assumes that all read bytes are stored in contiguous memory locations. It makes no sense to weaken this requirement and make decoding ~1.5-3x slower because of that. NFD is already slow enough. If we can take advantage of the current de facto requirement of contiguous memory, we should do it.

memcpy: https://godbolt.org/g/bGJees
unaligned access: https://godbolt.org/g/6Cd49X

Both gcc and clang generate exactly the same assembly code in the two cases. This is true for 2-, 4-, and 8-byte reads, and on x86_64, arm64 (aka aarch64), ppc, and ppc64. I suppose that means the compiler knows that unaligned accesses are safe (and efficient) on those architectures, and optimizes the memcpy appropriately.