[PATCH] D104076: [clang-cl][sanitizer] Add -fsanitize-address-use-after-return to clang.
Vitaly Buka via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 10 22:43:41 PDT 2021
vitalybuka accepted this revision.
vitalybuka added a comment.
This revision is now accepted and ready to land.
LGTM with some nits and if you extract FunctionStackPoisoner::initializeCallbacks into a separate patch
================
Comment at: clang/test/CodeGen/asan-use-after-return.cpp:3
+// RUN: | FileCheck %s --check-prefixes=CHECK-RUNTIME \
+// RUN: --implicit-check-not="call{{.*}}__asan_stack_malloc_always_"
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-linux %s \
----------------
I guess call{{.*}} can be removed from implicit-check-not?
================
Comment at: clang/test/Driver/fsanitize-use-after-return.c:17-20
+// RUN: %clang -target x86_64-apple-macosx10.15-gnu -fsanitize=address \
+// RUN: -fsanitize-address-use-after-return=always %s -### 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-ALWAYS-ARG %s
+// CHECK-ALWAYS-ARG: "-fsanitize-address-use-after-return=always"
----------------
we also want test like this:
```
// RUN: %clang -target x86_64-apple-macosx10.15-gnu -fsanitize=address \
// RUN: -fsanitize-address-use-after-return=never -fsanitize-address-use-after-return=always %s -### 2>&1 | \
// RUN: FileCheck -check-prefix=CHECK-ALWAYS %s
```
================
Comment at: llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h:105
+ bool UseAfterScope = false,
+ llvm::AsanDetectStackUseAfterReturnMode UseAfterReturn =
+ llvm::AsanDetectStackUseAfterReturnMode::Never);
----------------
it's already in llvm:: namespace
================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:637
+ bool UseAfterScope = false,
+ llvm::AsanDetectStackUseAfterReturnMode UseAfterReturn =
+ llvm::AsanDetectStackUseAfterReturnMode::Never)
----------------
-llvm::
================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:652-654
+ if (ClUseAfterReturn != ClUseAfterReturn.getDefault().getValue()) {
+ this->UseAfterReturn = ClUseAfterReturn;
+ }
----------------
CL should override argument even if it's default:
maybe at line 643:
UseAfterReturn(ClUseAfterReturn.getNumOccurrences() ? ClUseAfterReturn : UseAfterReturn)
================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:771
+ bool UseAfterScope = false,
+ llvm::AsanDetectStackUseAfterReturnMode UseAfterReturn =
+ llvm::AsanDetectStackUseAfterReturnMode::Never)
----------------
drop llvm::
================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2973
IRBuilder<> IRB(*C);
- switch (ClUseAfterReturn) {
- case AsanDetectStackUseAfterReturnMode::Always:
- for (int i = 0; i <= kMaxAsanStackMallocSizeClass; i++) {
- std::string Suffix = itostr(i);
- AsanStackMallocFunc[i] = M.getOrInsertFunction(
- kAsanStackMallocAlwaysNameTemplate + Suffix, IntptrTy, IntptrTy);
- AsanStackFreeFunc[i] =
- M.getOrInsertFunction(kAsanStackFreeNameTemplate + Suffix,
- IRB.getVoidTy(), IntptrTy, IntptrTy);
- }
- break;
- case AsanDetectStackUseAfterReturnMode::Runtime:
- for (int i = 0; i <= kMaxAsanStackMallocSizeClass; i++) {
- std::string Suffix = itostr(i);
- AsanStackMallocFunc[i] = M.getOrInsertFunction(
- kAsanStackMallocNameTemplate + Suffix, IntptrTy, IntptrTy);
- AsanStackFreeFunc[i] =
- M.getOrInsertFunction(kAsanStackFreeNameTemplate + Suffix,
- IRB.getVoidTy(), IntptrTy, IntptrTy);
- }
- break;
- case AsanDetectStackUseAfterReturnMode::Never:
- // Do Nothing
- break;
- case AsanDetectStackUseAfterReturnMode::Invalid:
- // Do Nothing
- break;
+ const char *kMallocNameTemplate = kAsanStackMallocNameTemplate;
+ if (ASan.UseAfterReturn == AsanDetectStackUseAfterReturnMode::Always) {
----------------
it looks like unrelated patch
================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2973
IRBuilder<> IRB(*C);
- switch (ClUseAfterReturn) {
- case AsanDetectStackUseAfterReturnMode::Always:
- for (int i = 0; i <= kMaxAsanStackMallocSizeClass; i++) {
- std::string Suffix = itostr(i);
- AsanStackMallocFunc[i] = M.getOrInsertFunction(
- kAsanStackMallocAlwaysNameTemplate + Suffix, IntptrTy, IntptrTy);
- AsanStackFreeFunc[i] =
- M.getOrInsertFunction(kAsanStackFreeNameTemplate + Suffix,
- IRB.getVoidTy(), IntptrTy, IntptrTy);
- }
- break;
- case AsanDetectStackUseAfterReturnMode::Runtime:
- for (int i = 0; i <= kMaxAsanStackMallocSizeClass; i++) {
- std::string Suffix = itostr(i);
- AsanStackMallocFunc[i] = M.getOrInsertFunction(
- kAsanStackMallocNameTemplate + Suffix, IntptrTy, IntptrTy);
- AsanStackFreeFunc[i] =
- M.getOrInsertFunction(kAsanStackFreeNameTemplate + Suffix,
- IRB.getVoidTy(), IntptrTy, IntptrTy);
- }
- break;
- case AsanDetectStackUseAfterReturnMode::Never:
- // Do Nothing
- break;
- case AsanDetectStackUseAfterReturnMode::Invalid:
- // Do Nothing
- break;
+ const char *kMallocNameTemplate = kAsanStackMallocNameTemplate;
+ if (ASan.UseAfterReturn == AsanDetectStackUseAfterReturnMode::Always) {
----------------
vitalybuka wrote:
> it looks like unrelated patch
could you please fix clang-tidy: warnings
================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2973
IRBuilder<> IRB(*C);
- switch (ClUseAfterReturn) {
- case AsanDetectStackUseAfterReturnMode::Always:
- for (int i = 0; i <= kMaxAsanStackMallocSizeClass; i++) {
- std::string Suffix = itostr(i);
- AsanStackMallocFunc[i] = M.getOrInsertFunction(
- kAsanStackMallocAlwaysNameTemplate + Suffix, IntptrTy, IntptrTy);
- AsanStackFreeFunc[i] =
- M.getOrInsertFunction(kAsanStackFreeNameTemplate + Suffix,
- IRB.getVoidTy(), IntptrTy, IntptrTy);
- }
- break;
- case AsanDetectStackUseAfterReturnMode::Runtime:
- for (int i = 0; i <= kMaxAsanStackMallocSizeClass; i++) {
- std::string Suffix = itostr(i);
- AsanStackMallocFunc[i] = M.getOrInsertFunction(
- kAsanStackMallocNameTemplate + Suffix, IntptrTy, IntptrTy);
- AsanStackFreeFunc[i] =
- M.getOrInsertFunction(kAsanStackFreeNameTemplate + Suffix,
- IRB.getVoidTy(), IntptrTy, IntptrTy);
- }
- break;
- case AsanDetectStackUseAfterReturnMode::Never:
- // Do Nothing
- break;
- case AsanDetectStackUseAfterReturnMode::Invalid:
- // Do Nothing
- break;
+ const char *kMallocNameTemplate = kAsanStackMallocNameTemplate;
+ if (ASan.UseAfterReturn == AsanDetectStackUseAfterReturnMode::Always) {
----------------
vitalybuka wrote:
> vitalybuka wrote:
> > it looks like unrelated patch
> could you please fix clang-tidy: warnings
now we insert functions also for Never and Invalid?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104076/new/
https://reviews.llvm.org/D104076
More information about the llvm-commits
mailing list