MaskRay added inline comments.
================ Comment at: compiler-rt/lib/asan_abi/asan_abi_shim.cpp:14 +extern "C" { +// Globals +void __asan_register_image_globals(uptr *flag) { ---------------- Comments are usually a complete sentence with a period. There are exceptions, but a "Globals" needs elaboration to make it better understandable by a reader. ================ Comment at: compiler-rt/lib/asan_abi/asan_abi_shim.cpp:16 +void __asan_register_image_globals(uptr *flag) { + __asan_abi_register_image_globals(); +} ---------------- 2-space indentation, here and throughout. ================ Comment at: compiler-rt/lib/asan_abi/asan_abi_shim.cpp:41 + +void __asan_after_dynamic_init(void) { + __asan_abi_after_dynamic_init(); ---------------- C++ prefers `()` instead of `(void)`. ================ Comment at: compiler-rt/lib/asan_abi/asan_abi_shim.cpp:486 + // TBD: Fail here + return (void *) 0; +} ---------------- You may apply `clang-format`, which may turn this into `(void *)0`, but `nullptr` likely works better. ================ Comment at: compiler-rt/test/asan_abi/TestCases/Darwin/llvm_interface_symbols.cpp:1 +// RUN: %clang_asan_abi -O2 -c -fsanitize-stable-abi -fsanitize=address -O0 %s -o %t.o +// RUN: %clangxx -c %p/../../../../lib/asan_abi/asan_abi.cpp -o asan_abi.o ---------------- excess spaces `-O2 ... -O0`? ================ Comment at: compiler-rt/test/asan_abi/TestCases/Darwin/llvm_interface_symbols.cpp:4 +// RUN: %clangxx -dead_strip -o %t %t.o %libasan_abi asan_abi.o && %run %t 2>&1 + + ---------------- One blank line suffices. ================ Comment at: compiler-rt/test/asan_abi/TestCases/Darwin/llvm_interface_symbols.cpp:9 + +// RUN: nm -g %libasan_abi \ +// RUN: | grep " [TU] " \ ---------------- We generally prefer llvm counterparts to the system binary utilities. Use `llvm-nm` ================ Comment at: compiler-rt/test/asan_abi/TestCases/Darwin/llvm_interface_symbols.cpp:15 +// RUN: > %t.exports +// +// RUN: sed -e ':a' -e 'N' -e '$!ba' \ ---------------- unneeded `^//$` lines, here and throughout. ================ Comment at: compiler-rt/test/asan_abi/TestCases/Darwin/llvm_interface_symbols.cpp:16 +// +// RUN: sed -e ':a' -e 'N' -e '$!ba' \ +// RUN: -e 's/ //g' \ ---------------- Does Darwin sed accept `;` to combine multiple `-e` into one `-e` with `;`? ================ Comment at: compiler-rt/test/asan_abi/TestCases/Darwin/llvm_interface_symbols.cpp:21 +// RUN: %t.asan_interface.inc \ +// RUN: | grep -v -f %p/../../../../lib/asan_abi/darwin_exclude_symbols.inc \ +// RUN: | grep -e "INTERFACE_\(WEAK_\)\?FUNCTION" \ ---------------- 2-space indentation for `|` continuation lines as well ================ Comment at: compiler-rt/test/asan_abi/TestCases/Darwin/llvm_interface_symbols.cpp:26 +// +// RUN: cat %t.imports | sort | uniq > %t.imports-sorted +// RUN: cat %t.exports | sort | uniq > %t.exports-sorted ---------------- `sort %t.imports`. See `Useless use of cat` ================ Comment at: compiler-rt/test/asan_abi/TestCases/linkstaticlibrary.cpp:1 +// RUN: %clang_asan_abi -O2 -c -fsanitize-stable-abi -fsanitize=address -O0 %s -o %t.o +// RUN: %clangxx -c %p/../../../lib/asan_abi/asan_abi.cpp -o asan_abi.o ---------------- `-O2 ... -O0` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits