search

dgibson / dtc

Device Tree Compiler
226 stars 133 forks source link

Should use strcmp/strncmp instead of memcmp on arm. #23

Closed x-projs closed 5 years ago

x-projs commented 5 years ago

In libfdt code, seems we use memcmp to compare two strings in some places. It is not correct behavior on arm. Arm has memory alignment requirement on memcmp(). But for string, especially the string directly comes from dt string block, there is no guarantee that the address is aligned. memcmp() will cause an exception.

dgibson commented 5 years ago

Um.. a memcmp() implementation which has alignment constraints is utterly broken. I can't believe that's the case on all ARM systems - what specific environment are you using?

x-projs commented 5 years ago

compiler: gcc-arm-8.2-2018.08-x86_64-aarch64-elf which uses newlib device: Raspberry Pi 3 B

x-projs commented 5 years ago

S:\src\pios>gcc-arm-8.2-2018.08-i686-mingw32-aarch64-elf\bin\aarch64-elf-gcc -v Using built-in specs. COLLECT_GCC=gcc-arm-8.2-2018.08-i686-mingw32-aarch64-elf\bin\aarch64-elf-gcc COLLECT_LTO_WRAPPER=s:/src/pios/gcc-arm-8.2-2018.08-i686-mingw32-aarch64-elf/bin/../libexec/gcc/aarch64-elf/8.2.1/lto-wrapper.exe Target: aarch64-elf Configured with: /tmp/dgboter/bbs/dsggnu-vm-1-x86_64--mingw32-i686/buildbot/mingw32-i686--aarch64-elf/build/src/gcc/configure --target=aarch64-elf --prefix=/tmp/dgboter/bbs/dsggnu-vm-1-x86_64--mingw32-i686/buildbot/mingw32-i686--aarch64-elf/build/build-mingw-aarch64-elf/install// --with-gmp=/tmp/dgboter/bbs/dsggnu-vm-1-x86_64--mingw32-i686/buildbot/mingw32-i686--aarch64-elf/build/build-mingw-aarch64-elf/host-tools --with-mpfr=/tmp/dgboter/bbs/dsggnu-vm-1-x86_64--mingw32-i686/buildbot/mingw32-i686--aarch64-elf/build/build-mingw-aarch64-elf/host-tools --with-mpc=/tmp/dgboter/bbs/dsggnu-vm-1-x86_64--mingw32-i686/buildbot/mingw32-i686--aarch64-elf/build/build-mingw-aarch64-elf/host-tools --with-isl=/tmp/dgboter/bbs/dsggnu-vm-1-x86_64--mingw32-i686/buildbot/mingw32-i686--aarch64-elf/build/build-mingw-aarch64-elf/host-tools --with-pkgversion='GNU Toolchain for the A-profile Architecture 8.2-2018-08 (arm-rel-8.23)' --disable-shared --disable-nls --disable-threads --disable-tls --enable-checking=release --enable-languages=c,c++,fortran --with-newlib --with-libiconv-prefix=/tmp/dgboter/bbs/dsggnu-vm-1-x86_64--mingw32-i686/buildbot/mingw32-i686--aarch64-elf/build/build-mingw-aarch64-elf/host-tools --host=i686-w64-mingw32 Thread model: single gcc version 8.2.1 20180802 (GNU Toolchain for the A-profile Architecture 8.2-2018-08 (arm-rel-8.23))

dgibson commented 5 years ago

Ok, it sounds like that newlib version may be broken for ARM. I'm 99% certain that a memcmp() which doesn't work on unaligned addresses is a serious spec violation.

dgibson commented 5 years ago

Checked with some people who know C specs better than I do. A memcmp() that doesn't handle non-aligned addresses is definitely broken. You'll either need to get the bug fixed in your library, or you'll need to supply a working memcmp() via a custom libfdt_env.h.