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
