[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-22 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

In D105703#2898350 , @vitalybuka 
wrote:

> In D105703#2898343 , @vitalybuka 
> wrote:
>
>> fix crash
>
> weird, my config with -DLLVM_ENABLE_ASSERTIONS=ON does not reproduce, however 
> exact cmake from bot does

-DLLVM_TARGETS_TO_BUILD=X86 was needed as well


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-22 Thread Vitaly Buka via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG96c63492cb95: [hwasan] Use stack safety analysis. (authored 
by fmayer, committed by vitalybuka).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -204,11 +210,23 @@
   return ClInstrumentWithCalls || TargetTriple.getArch() == Triple::x86_64;
 }
 
+bool mightUseStackSafetyAnalysis(bool DisableOptimization) {
+  return ClUseStackSafety.getNumOccurrences() ? ClUseStackSafety
+  : !DisableOptimization;
+}
+
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  return shouldInstrumentStack(TargetTriple) &&
+ mightUseStackSafetyAnalysis(DisableOptimization);
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+ const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -217,6 +235,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-22 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

In D105703#2898343 , @vitalybuka 
wrote:

> fix crash

weird, my config with -DLLVM_ENABLE_ASSERTIONS=ON does not reproduce, however 
exact cmake from bot does


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-22 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka updated this revision to Diff 361002.
vitalybuka added a comment.

fix crash


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -204,11 +210,23 @@
   return ClInstrumentWithCalls || TargetTriple.getArch() == Triple::x86_64;
 }
 
+bool mightUseStackSafetyAnalysis(bool DisableOptimization) {
+  return ClUseStackSafety.getNumOccurrences() ? ClUseStackSafety
+  : !DisableOptimization;
+}
+
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  return shouldInstrumentStack(TargetTriple) &&
+ mightUseStackSafetyAnalysis(DisableOptimization);
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+ const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -217,6 +235,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -269,6 +289,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-22 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:414
 
 INITIALIZE_PASS_BEGIN(
 HWAddressSanitizerLegacyPass, "hwasan",

To fix that revert you need to add 

INITIALIZE_PASS_DEPENDENCY(StackSafetyGlobalInfoWrapperPass)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-22 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

Do you build with assertions enabled?

In D105703#2896056 , @fmayer wrote:

> Sorry. broke a buildbot again: 
> https://lab.llvm.org/buildbot/#/builders/139/builds/7613/steps/6/logs/FAIL__Clang__asan_c
>
> I am not sure why I cannot reproduce this locally.
>
>   Pass 'HWAddressSanitizer' is not initialized.
>   Verify if there is a pass dependency cycle.
>   Required Passes:
>   clang: 
> /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:715:
>  void llvm::PMTopLevelManager::schedulePass(llvm::Pass*): Assertion `PI && 
> "Expected required passes to be initialized"' failed.
>   PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
> backtrace, preprocessed source, and associated run script.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-22 Thread Florian Mayer via Phabricator via cfe-commits
fmayer reopened this revision.
fmayer added a comment.
This revision is now accepted and ready to land.

Sorry. broke a buildbot again: 
https://lab.llvm.org/buildbot/#/builders/139/builds/7613/steps/6/logs/FAIL__Clang__asan_c

I am not sure why I cannot reproduce this locally.

  Pass 'HWAddressSanitizer' is not initialized.
  Verify if there is a pass dependency cycle.
  Required Passes:
  clang: 
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:715:
 void llvm::PMTopLevelManager::schedulePass(llvm::Pass*): Assertion `PI && 
"Expected required passes to be initialized"' failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-22 Thread Florian Mayer via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
fmayer marked 4 inline comments as done.
Closed by commit rGbde9415fef25: [hwasan] Use stack safety analysis. (authored 
by fmayer).

Changed prior to commit:
  https://reviews.llvm.org/D105703?vs=360735=360761#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -204,11 +210,23 @@
   return ClInstrumentWithCalls || TargetTriple.getArch() == Triple::x86_64;
 }
 
+bool mightUseStackSafetyAnalysis(bool DisableOptimization) {
+  return ClUseStackSafety.getNumOccurrences() ? ClUseStackSafety
+  : !DisableOptimization;
+}
+
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  return shouldInstrumentStack(TargetTriple) &&
+ mightUseStackSafetyAnalysis(DisableOptimization);
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+ const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -217,6 +235,8 @@
 initializeModule();
   }

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-22 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 360735.
fmayer added a comment.

formatting.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -216,11 +222,23 @@
 #endif
 }
 
+bool mightUseStackSafetyAnalysis(bool DisableOptimization) {
+  return ClUseStackSafety.getNumOccurrences() ? ClUseStackSafety
+  : !DisableOptimization;
+}
+
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  return shouldInstrumentStack(TargetTriple) &&
+ mightUseStackSafetyAnalysis(DisableOptimization);
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+ const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -229,6 +247,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -281,6 +301,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-21 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka accepted this revision.
vitalybuka added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:409
+// analysis.
+// This is so we don't need to plumb TargetTriple all the way to here.
+if (mightUseStackSafetyAnalysis(DisableOptimization))

I think this function without comment is clear enough. But up to you.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:410
+Dummy.setArch(Triple::aarch64);
+if (shouldUseStackSafetyAnalysis(Dummy, DisableOptimization))
+  AU.addRequired();

fmayer wrote:
> fmayer wrote:
> > vitalybuka wrote:
> > > Why not just:
> > > ```
> > > if (!DisableOptimization || ClUseStackSafety)
> > >   AU.addRequired();
> > > ```
> > Because `ClUseStackSafety` defaults to true, so this will be quite useless 
> > except if the user explicitly sets it to false. So then you end up 
> > duplicating the logic in `shouldUseStackSafetyAnalysis`, which is why I 
> > went for the Dummy (thinking about it, we don't actually need to setArch on 
> > it).
> How about this (mightUseStackSafetyAnalysis)? This seems like a nice tradeoff 
> without duplication of the logic.
LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-21 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:410
+Dummy.setArch(Triple::aarch64);
+if (shouldUseStackSafetyAnalysis(Dummy, DisableOptimization))
+  AU.addRequired();

fmayer wrote:
> vitalybuka wrote:
> > Why not just:
> > ```
> > if (!DisableOptimization || ClUseStackSafety)
> >   AU.addRequired();
> > ```
> Because `ClUseStackSafety` defaults to true, so this will be quite useless 
> except if the user explicitly sets it to false. So then you end up 
> duplicating the logic in `shouldUseStackSafetyAnalysis`, which is why I went 
> for the Dummy (thinking about it, we don't actually need to setArch on it).
How about this (mightUseStackSafetyAnalysis)? This seems like a nice tradeoff 
without duplication of the logic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-21 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 360623.
fmayer added a comment.

more flag parsing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -216,11 +222,22 @@
 #endif
 }
 
+bool mightUseStackSafetyAnalysis(bool DisableOptimization) {
+  return ClUseStackSafety.getNumOccurrences() ? ClUseStackSafety
+  : !DisableOptimization;
+}
+
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  return shouldInstrumentStack(TargetTriple) && mightUseStackSafetyAnalysis(DisableOptimization);
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+ const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -229,6 +246,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -281,6 +300,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-21 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:410
+Dummy.setArch(Triple::aarch64);
+if (shouldUseStackSafetyAnalysis(Dummy, DisableOptimization))
+  AU.addRequired();

vitalybuka wrote:
> Why not just:
> ```
> if (!DisableOptimization || ClUseStackSafety)
>   AU.addRequired();
> ```
Because `ClUseStackSafety` defaults to true, so this will be quite useless 
except if the user explicitly sets it to false. So then you end up duplicating 
the logic in `shouldUseStackSafetyAnalysis`, which is why I went for the Dummy 
(thinking about it, we don't actually need to setArch on it).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-21 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:410
+Dummy.setArch(Triple::aarch64);
+if (shouldUseStackSafetyAnalysis(Dummy, DisableOptimization))
+  AU.addRequired();

Why not just:
```
if (!DisableOptimization || ClUseStackSafety)
  AU.addRequired();
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-21 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:406
+// architecture that doesn't allow stack tagging we will still load the
+// analysis.
+// This is so we don't need to plumb TargetTriple all the way to here.

vitalybuka wrote:
> I like this even less than "plumbing",  if you need 
> shouldUseStackSafetyAnalysis please just pass Triple as in earlier versions.
> 
> Why do you need to avoid loading analysis? Revert does not explain what is 
> broken.
I am still looking at the breakage. But if  you looked at the logic before, 
there was the case DisableOptimization BUT ClUseStackAnalysis being explicitly 
set, which made us try to getAnalysis without having required it here.

I also like the plumbing more than this, but changed to this to see what you 
think.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-21 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added a comment.

This broke https://lab.llvm.org/buildbot/#/builders/105/builds/12584.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-20 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:406
+// architecture that doesn't allow stack tagging we will still load the
+// analysis.
+// This is so we don't need to plumb TargetTriple all the way to here.

I like this even less than "plumbing",  if you need 
shouldUseStackSafetyAnalysis please just pass Triple as in earlier versions.

Why do you need to avoid loading analysis? Revert does not explain what is 
broken.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-20 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 360076.
fmayer added a comment.

fix required analysis logic


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -216,11 +222,20 @@
 #endif
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+ const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -229,6 +244,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -281,6 +298,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
@@ -351,7 +369,8 @@
   static 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-20 Thread Florian Mayer via Phabricator via cfe-commits
fmayer reopened this revision.
fmayer added a comment.
This revision is now accepted and ready to land.

Made some buildbot unhappy again. Sorry :(


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-20 Thread Florian Mayer via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe9c63ed10b3b: [hwasan] Use stack safety analysis. (authored 
by fmayer).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -216,11 +222,22 @@
 #endif
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+ const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -229,6 +246,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -281,6 +300,7 @@
 private:
   LLVMContext 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-20 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 360054.
fmayer marked an inline comment as done.
fmayer added a comment.

format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -216,11 +222,22 @@
 #endif
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+ const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -229,6 +246,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -281,6 +300,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-19 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis accepted this revision.
eugenis added a comment.

In D105703#2887005 , @fmayer wrote:

> I removed the stack-safety-analysis-asm.c test because I don't think it 
> really adds anything and it caused problems. SGTY?

Absolutely.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-19 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added a comment.

I removed the stack-safety-analysis-asm.c test because I don't think it really 
adds anything and it caused problems. SGTY?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-19 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 359749.
fmayer added a comment.

remove stack safety asm test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -216,11 +222,22 @@
 #endif
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+  const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -229,6 +246,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -281,6 +300,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-19 Thread Florian Mayer via Phabricator via cfe-commits
fmayer reopened this revision.
fmayer added a comment.
This revision is now accepted and ready to land.

Broke some postsubmit bot.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-19 Thread Florian Mayer via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG12268fe14a1a: [hwasan] Use stack safety analysis. (authored 
by fmayer).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -216,11 +222,22 @@
 #endif
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+  const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -229,6 +246,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-19 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 359726.
fmayer added a comment.

rebase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -216,11 +222,22 @@
 #endif
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  HWAddressSanitizer(Module , bool CompileKernel, bool Recover) : M(M) {
+  HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+  const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -229,6 +246,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -281,6 +300,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-19 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added inline comments.



Comment at: 
llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll:27
+
+attributes #0 = { mustprogress nofree nounwind uwtable willreturn 
"frame-pointer"="non-leaf" "min-legal-vector-width"="0" 
"no-trapping-math"="true" "stack-protector-buffer-size"="8" 
"target-cpu"="generic" "target-features"="+neon" }
+attributes #1 = { argmemonly mustprogress nofree nosync nounwind willreturn }

vitalybuka wrote:
> usually we remove irrelevant attributes #* and module flags !* to make test 
> simpler
Removed all sorts of unneeded stuff. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-19 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 359716.
fmayer added a comment.

update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture)
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,22 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  explicit HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+  const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -225,6 +240,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -277,6 +294,7 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-19 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 359715.
fmayer added a comment.

style fix


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+declare void @use(i8* nocapture) #1
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,22 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  explicit HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+  const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -225,6 +240,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -277,6 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-19 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 359714.
fmayer marked 6 inline comments as done.
fmayer added a comment.

improve test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,42 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Check a safe alloca to ensure it does not get a tag.
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Check a non-safe alloca to ensure it gets a tag.
+define i32 @test_use(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @use(i8* nonnull %buf.sroa.0)
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #1
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #1
+
+declare void @use(i8* nocapture) #1
+
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,22 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  explicit HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+  const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -225,6 +240,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:427
+ bool DisableOptimization) {
   assert(!CompileKernel || Recover);
+  return new HWAddressSanitizerLegacyPass(CompileKernel, Recover,

eugenis wrote:
> vitalybuka wrote:
> > @eugenis unrelated to the patch, but why do we this args if then we 
> > assert(!CompileKernel || Recover);
> why not? This forbids CompileKernel && !Recover, how else would you represent 
> the 3 remaining combinations?
> why not? This forbids CompileKernel && !Recover, how else would you represent 
> the 3 remaining combinations?

Thanks. I've misread the expression.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis added a comment.

In D105703#2884056 , @vitalybuka 
wrote:

> In D105703#2883666 , @eugenis wrote:
>
>> Btw Vitaly, will this work with LTO out of the box? I think we used to add 
>> pre-LTO StackSafety pass explicitly for memtag only, but it looks like that 
>> code is gone.
>
> What do you mean gone? Can you show me the patch?
>
> We probably needs something to add analysis for HWASAN as well.

https://reviews.llvm.org/D80771




Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:427
+ bool DisableOptimization) {
   assert(!CompileKernel || Recover);
+  return new HWAddressSanitizerLegacyPass(CompileKernel, Recover,

vitalybuka wrote:
> @eugenis unrelated to the patch, but why do we this args if then we 
> assert(!CompileKernel || Recover);
why not? This forbids CompileKernel && !Recover, how else would you represent 
the 3 remaining combinations?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

In D105703#2883666 , @eugenis wrote:

> Btw Vitaly, will this work with LTO out of the box? I think we used to add 
> pre-LTO StackSafety pass explicitly for memtag only, but it looks like that 
> code is gone.

What do you mean gone? Can you show me the patch?

We probably needs something to add analysis for HWASAN as well.




Comment at: llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h:32
+  Triple TargetTriple = {});
   PreservedAnalyses run(Module , ModuleAnalysisManager );
   static bool isRequired() { return true; }

fmayer wrote:
> vitalybuka wrote:
> > fmayer wrote:
> > > vitalybuka wrote:
> > > > fmayer wrote:
> > > > > vitalybuka wrote:
> > > > > > Why not from M.getTargetTriple() ?
> > > > > Mostly for consistency with the legacy pass. Either way is fine for 
> > > > > me though, what do you prefer?
> > > > I don't know if will cause any issues, but usually most passes get 
> > > > triple from the module.
> > > > I prefer we stay consistent with the rest of the code if possible.
> > > > 
> > > I'll leave it as is, for consistency within this file, as we need to do 
> > > it this way for the new pass manager.
> > That's what I don't like, passing Triple from BackendUtil.cpp
> > I've updated the patch. You can download it with "arc patch D105703"
> OK, no strong feelings but now we addRequire the pass even if we don't need 
> it, so there isn't much point turning it off anymore, no?
In case of legacy pass? Correct. I don't think we need to over optimize this 
deprecated case which is going to be removed at some point anyway.
Let's check DisableOptimization as we have it here anyway, but avoid forwarding 
Triple.

For new PM it's irrelevant as it's does not declare requirements in advance.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:427
+ bool DisableOptimization) {
   assert(!CompileKernel || Recover);
+  return new HWAddressSanitizerLegacyPass(CompileKernel, Recover,

@eugenis unrelated to the patch, but why do we this args if then we 
assert(!CompileKernel || Recover);



Comment at: 
llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll:5
+; ModuleID = 
'/usr/local/google/home/fmayer/llvm-project/clang/test/CodeGen/hwasan-stack-safety-analysis.c'
+source_filename = 
"/usr/local/google/home/fmayer/llvm-project/clang/test/CodeGen/hwasan-stack-safety-analysis.c"
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"

you don't need to hardcode local path here



Comment at: 
llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll:10
+; Function Attrs: mustprogress nofree nounwind uwtable willreturn
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:

would be nice to have also function which have __hwasan_generate_tag in either 
case to make sure 
-hwasan-use-stack-safety=1 does not disable tagging completely.



Comment at: 
llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll:27
+
+attributes #0 = { mustprogress nofree nounwind uwtable willreturn 
"frame-pointer"="non-leaf" "min-legal-vector-width"="0" 
"no-trapping-math"="true" "stack-protector-buffer-size"="8" 
"target-cpu"="generic" "target-features"="+neon" }
+attributes #1 = { argmemonly mustprogress nofree nosync nounwind willreturn }

usually we remove irrelevant attributes #* and module flags !* to make test 
simpler


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis added a comment.

Btw Vitaly, will this work with LTO out of the box? I think we used to add 
pre-LTO StackSafety pass explicitly for memtag only, but it looks like that 
code is gone.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 359376.
fmayer marked 3 inline comments as done.
fmayer added a comment.

fix comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,43 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+; ModuleID = '/usr/local/google/home/fmayer/llvm-project/clang/test/CodeGen/hwasan-stack-safety-analysis.c'
+source_filename = "/usr/local/google/home/fmayer/llvm-project/clang/test/CodeGen/hwasan-stack-safety-analysis.c"
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Function Attrs: mustprogress nofree nounwind uwtable willreturn
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #1
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #1
+
+attributes #0 = { mustprogress nofree nounwind uwtable willreturn "frame-pointer"="non-leaf" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+neon" }
+attributes #1 = { argmemonly mustprogress nofree nosync nounwind willreturn }
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 1, !"branch-target-enforcement", i32 0}
+!2 = !{i32 1, !"sign-return-address", i32 0}
+!3 = !{i32 1, !"sign-return-address-all", i32 0}
+!4 = !{i32 1, !"sign-return-address-with-bkey", i32 0}
+!5 = !{i32 7, !"uwtable", i32 1}
+!6 = !{i32 7, !"frame-pointer", i32 1}
+!7 = !{!"clang version 13.0.0 (/usr/local/google/home/fmayer/llvm-project/clang 267dacd628370863dd960ba41b664b6f86c56961)"}
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,22 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  explicit 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis accepted this revision.
eugenis added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1174
+CompileKernel, Recover,
+/*IsOptNull=*/CodeGenOpts.OptimizationLevel == 0));
   }

DisableOptimization=



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:407
   bool Recover;
+  bool DisableOptimization;
 };

fmayer wrote:
> fmayer wrote:
> > eugenis wrote:
> > > No need to pass this down, just look at OptimizeNone function attribute.
> > Interesting, the Aarch64StackTagging code does pass this down, do you know 
> > why?
> Also we really should be using this at least in the getAnalysisUsage, which 
> Vitaly's change made unconditional. Correct?
Ah right. Legacy pass needs to declare its analysis dependencies in advance.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 359373.
fmayer added a comment.

don't use stack analysis for opt null


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,43 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+; ModuleID = '/usr/local/google/home/fmayer/llvm-project/clang/test/CodeGen/hwasan-stack-safety-analysis.c'
+source_filename = "/usr/local/google/home/fmayer/llvm-project/clang/test/CodeGen/hwasan-stack-safety-analysis.c"
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Function Attrs: mustprogress nofree nounwind uwtable willreturn
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #1
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #1
+
+attributes #0 = { mustprogress nofree nounwind uwtable willreturn "frame-pointer"="non-leaf" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+neon" }
+attributes #1 = { argmemonly mustprogress nofree nosync nounwind willreturn }
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 1, !"branch-target-enforcement", i32 0}
+!2 = !{i32 1, !"sign-return-address", i32 0}
+!3 = !{i32 1, !"sign-return-address-all", i32 0}
+!4 = !{i32 1, !"sign-return-address-with-bkey", i32 0}
+!5 = !{i32 7, !"uwtable", i32 1}
+!6 = !{i32 7, !"frame-pointer", i32 1}
+!7 = !{!"clang version 13.0.0 (/usr/local/google/home/fmayer/llvm-project/clang 267dacd628370863dd960ba41b664b6f86c56961)"}
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,22 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  explicit 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:407
   bool Recover;
+  bool DisableOptimization;
 };

fmayer wrote:
> eugenis wrote:
> > No need to pass this down, just look at OptimizeNone function attribute.
> Interesting, the Aarch64StackTagging code does pass this down, do you know 
> why?
Also we really should be using this at least in the getAnalysisUsage, which 
Vitaly's change made unconditional. Correct?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Florian Mayer via Phabricator via cfe-commits
fmayer marked 2 inline comments as done.
fmayer added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:407
   bool Recover;
+  bool DisableOptimization;
 };

eugenis wrote:
> No need to pass this down, just look at OptimizeNone function attribute.
Interesting, the Aarch64StackTagging code does pass this down, do you know why?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:407
   bool Recover;
+  bool DisableOptimization;
 };

No need to pass this down, just look at OptimizeNone function attribute.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Florian Mayer via Phabricator via cfe-commits
fmayer marked 3 inline comments as done.
fmayer added inline comments.



Comment at: llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c:1
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm 
-hwasan-use-stack-safety=true -mllvm -hwasan-generate-tags-with-calls -O2 %s -o 
- | FileCheck %s --check-prefix=SAFETY
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm 
-hwasan-use-stack-safety=false -mllvm -hwasan-generate-tags-with-calls -O2 %s 
-o - | FileCheck %s --check-prefix=NOSAFETY

eugenis wrote:
> You can't use %clang in LLVM tests. There may be no clang. Use opt to invoke 
> the hwasan pass in isolation, see the tests under 
> test/Instrumentation/HWAddressSanitizer/.
> 
> The reason I did not say anything about the tests was because only in clang 
> we can test this thing end-to-end.
> Perhaps an LLVM test for instrumentation + a clang test for the passmanager 
> (use -fdebug-pass-manager and -mllvm -debug-pass=Structure for new/old).
> 
added an extra llvm test to check from IR, and left the existing clang end to 
end tests..



Comment at: llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c:8
+
+int main(int argc, char **argv) {
+  char buf[10];

vitalybuka wrote:
> these tests do not work because %clang is not defined here, in LLVM
> you can keep them but they need to stay in clang/
> 
> my request was to add new tests in 
> llvm-project/llvm/test/Instrumentation/HWAddressSanitizer/
> and they need to be *.ll tests
> you probably can generate them from C and cleanup manually
> or close existing *.ll tests and CHECK for expected difference if analysis is 
> enabled
> 
> Also try to generated CHECK statements with llvm/utils/update_test_checks.py, 
> often result is quite useful.
added llvm test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 359326.
fmayer added a comment.

add llvm test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
@@ -0,0 +1,43 @@
+; RUN: opt -hwasan -hwasan-use-stack-safety=1 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=SAFETY
+; RUN: opt -hwasan -hwasan-use-stack-safety=0 -hwasan-generate-tags-with-calls -S < %s | FileCheck %s --check-prefixes=NOSAFETY
+
+; ModuleID = '/usr/local/google/home/fmayer/llvm-project/clang/test/CodeGen/hwasan-stack-safety-analysis.c'
+source_filename = "/usr/local/google/home/fmayer/llvm-project/clang/test/CodeGen/hwasan-stack-safety-analysis.c"
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+; Function Attrs: mustprogress nofree nounwind uwtable willreturn
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  store volatile i8 0, i8* %buf.sroa.0, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #1
+
+; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #1
+
+attributes #0 = { mustprogress nofree nounwind uwtable willreturn "frame-pointer"="non-leaf" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+neon" }
+attributes #1 = { argmemonly mustprogress nofree nosync nounwind willreturn }
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 1, !"branch-target-enforcement", i32 0}
+!2 = !{i32 1, !"sign-return-address", i32 0}
+!3 = !{i32 1, !"sign-return-address-all", i32 0}
+!4 = !{i32 1, !"sign-return-address-with-bkey", i32 0}
+!5 = !{i32 7, !"uwtable", i32 1}
+!6 = !{i32 7, !"frame-pointer", i32 1}
+!7 = !{!"clang version 13.0.0 (/usr/local/google/home/fmayer/llvm-project/clang 267dacd628370863dd960ba41b664b6f86c56961)"}
+!8 = !{!9, !9, i64 0}
+!9 = !{!"omnipotent char", !10, i64 0}
+!10 = !{!"Simple C/C++ TBAA"}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,22 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  explicit HWAddressSanitizer(Module , bool 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 359263.
fmayer added a comment.

update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,22 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  explicit HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+  const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -225,6 +240,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -277,6 +294,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
@@ -347,7 +365,8 @@
   static char ID;
 
   explicit HWAddressSanitizerLegacyPass(bool CompileKernel = false,
-bool Recover = false)
+bool Recover = false,
+bool DisableOptimization = false)
   : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover) {
 initializeHWAddressSanitizerLegacyPassPass(
 *PassRegistry::getPassRegistry());
@@ -356,11 +375,19 @@
   StringRef getPassName() const override { return "HWAddressSanitizer"; }
 
   bool doInitialization(Module ) override {
-HWASan = std::make_unique(M, CompileKernel, Recover);
+HWASan = std::make_unique(M, CompileKernel, Recover,
+  /*SSI=*/nullptr);
 return true;
   }
 
   bool runOnFunction(Function ) override {
+if (shouldUseStackSafetyAnalysis(Triple(F.getParent()->getTargetTriple()),
+ DisableOptimization)) {
+  // We cannot call getAnalysis in doInitialization, that would cause a
+  // crash as the required analyses are not initialized yet.
+  HWASan->setSSI(
+  ().getResult());
+}
 return HWASan->sanitizeFunction(F);
   }
 
@@ -369,10 +396,15 @@
 return false;
   }
 
+  void getAnalysisUsage(AnalysisUsage ) const override {
+AU.addRequired();
+  }
+
 private:
   std::unique_ptr HWASan;
   bool CompileKernel;
   bool Recover;
+  bool DisableOptimization;
 };
 
 } // end anonymous namespace
@@ -388,18 +420,26 @@
 "HWAddressSanitizer: detect memory bugs using tagged addressing.", false,
 false)
 
-FunctionPass *llvm::createHWAddressSanitizerLegacyPassPass(bool CompileKernel,
-   bool Recover) {
+FunctionPass *
+llvm::createHWAddressSanitizerLegacyPassPass(bool 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-16 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added inline comments.



Comment at: llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h:32
+  Triple TargetTriple = {});
   PreservedAnalyses run(Module , ModuleAnalysisManager );
   static bool isRequired() { return true; }

vitalybuka wrote:
> fmayer wrote:
> > vitalybuka wrote:
> > > fmayer wrote:
> > > > vitalybuka wrote:
> > > > > Why not from M.getTargetTriple() ?
> > > > Mostly for consistency with the legacy pass. Either way is fine for me 
> > > > though, what do you prefer?
> > > I don't know if will cause any issues, but usually most passes get triple 
> > > from the module.
> > > I prefer we stay consistent with the rest of the code if possible.
> > > 
> > I'll leave it as is, for consistency within this file, as we need to do it 
> > this way for the new pass manager.
> That's what I don't like, passing Triple from BackendUtil.cpp
> I've updated the patch. You can download it with "arc patch D105703"
OK, no strong feelings but now we addRequire the pass even if we don't need it, 
so there isn't much point turning it off anymore, no?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-15 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

Please create *.ll tests, the rest is LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-15 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added inline comments.



Comment at: llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h:32
+  Triple TargetTriple = {});
   PreservedAnalyses run(Module , ModuleAnalysisManager );
   static bool isRequired() { return true; }

fmayer wrote:
> vitalybuka wrote:
> > fmayer wrote:
> > > vitalybuka wrote:
> > > > Why not from M.getTargetTriple() ?
> > > Mostly for consistency with the legacy pass. Either way is fine for me 
> > > though, what do you prefer?
> > I don't know if will cause any issues, but usually most passes get triple 
> > from the module.
> > I prefer we stay consistent with the rest of the code if possible.
> > 
> I'll leave it as is, for consistency within this file, as we need to do it 
> this way for the new pass manager.
That's what I don't like, passing Triple from BackendUtil.cpp
I've updated the patch. You can download it with "arc patch D105703"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-15 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka updated this revision to Diff 359154.
vitalybuka added a comment.

Remove triple


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -216,13 +222,22 @@
 #endif
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
-  explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  explicit HWAddressSanitizer(Module , bool CompileKernel, bool Recover,
+  const StackSafetyGlobalInfo *SSI)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -231,6 +246,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -283,6 +300,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
@@ -353,7 +371,8 @@
   static char ID;
 
   explicit HWAddressSanitizerLegacyPass(bool CompileKernel = false,
-bool Recover = false)
+bool Recover = false,
+bool DisableOptimization = false)
   : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover) {
 initializeHWAddressSanitizerLegacyPassPass(
 *PassRegistry::getPassRegistry());
@@ -362,11 +381,19 @@
   StringRef getPassName() const override { return "HWAddressSanitizer"; }
 
   bool doInitialization(Module ) override {
-HWASan = std::make_unique(M, CompileKernel, Recover);
+HWASan = std::make_unique(M, CompileKernel, Recover,
+  /*SSI=*/nullptr);
 return true;
   }
 
   bool runOnFunction(Function ) override {
+if (shouldUseStackSafetyAnalysis(Triple(F.getParent()->getTargetTriple()),
+ DisableOptimization)) {
+  // We cannot call getAnalysis in doInitialization, that would cause a
+  // crash as the required analyses are not initialized yet.
+  HWASan->setSSI(
+  ().getResult());
+}
 return HWASan->sanitizeFunction(F);
   }
 
@@ -375,10 +402,15 @@
 return false;
   }
 
+  void getAnalysisUsage(AnalysisUsage ) const override {
+AU.addRequired();
+  }
+
 private:
   std::unique_ptr HWASan;
   bool CompileKernel;
   bool Recover;
+  bool DisableOptimization;
 };
 
 } // end anonymous namespace
@@ -394,18 +426,26 @@
 "HWAddressSanitizer: detect memory bugs using tagged addressing.", false,
 false)
 
-FunctionPass *llvm::createHWAddressSanitizerLegacyPassPass(bool CompileKernel,
-   bool Recover) {
+FunctionPass *
+llvm::createHWAddressSanitizerLegacyPassPass(bool CompileKernel, bool 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-15 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added inline comments.



Comment at: llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c:8
+
+int main(int argc, char **argv) {
+  char buf[10];

these tests do not work because %clang is not defined here, in LLVM
you can keep them but they need to stay in clang/

my request was to add new tests in 
llvm-project/llvm/test/Instrumentation/HWAddressSanitizer/
and they need to be *.ll tests
you probably can generate them from C and cleanup manually
or close existing *.ll tests and CHECK for expected difference if analysis is 
enabled

Also try to generated CHECK statements with llvm/utils/update_test_checks.py, 
often result is quite useful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-15 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis added inline comments.



Comment at: llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c:1
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm 
-hwasan-use-stack-safety=true -mllvm -hwasan-generate-tags-with-calls -O2 %s -o 
- | FileCheck %s --check-prefix=SAFETY
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm 
-hwasan-use-stack-safety=false -mllvm -hwasan-generate-tags-with-calls -O2 %s 
-o - | FileCheck %s --check-prefix=NOSAFETY

You can't use %clang in LLVM tests. There may be no clang. Use opt to invoke 
the hwasan pass in isolation, see the tests under 
test/Instrumentation/HWAddressSanitizer/.

The reason I did not say anything about the tests was because only in clang we 
can test this thing end-to-end.
Perhaps an LLVM test for instrumentation + a clang test for the passmanager 
(use -fdebug-pass-manager and -mllvm -debug-pass=Structure for new/old).



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-15 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 358885.
fmayer added a comment.

fixup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  llvm/test/CodeGen/hwasan-stack-safety-analysis.c

Index: llvm/test/CodeGen/hwasan-stack-safety-analysis.c
===
--- /dev/null
+++ llvm/test/CodeGen/hwasan-stack-safety-analysis.c
@@ -0,0 +1,15 @@
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -emit-llvm -mllvm -hwasan-use-stack-safety=true -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=SAFETY
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -emit-llvm -mllvm -hwasan-use-stack-safety=false -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=NOSAFETY
+
+// Default when optimizing, but not with O0.
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -emit-llvm -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=SAFETY
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -emit-llvm -mllvm -hwasan-generate-tags-with-calls -O0 %s -o - | FileCheck %s --check-prefix=NOSAFETY
+
+int main(int argc, char **argv) {
+  char buf[10];
+  volatile char *x = buf;
+  *x = 0;
+  return buf[0];
+  // NOSAFETY: __hwasan_generate_tag
+  // SAFETY-NOT: __hwasan_generate_tag
+}
Index: llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c
===
--- /dev/null
+++ llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c
@@ -0,0 +1,15 @@
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm -hwasan-use-stack-safety=true -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=SAFETY
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm -hwasan-use-stack-safety=false -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=NOSAFETY
+
+// Default when optimizing, but not with O0.
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=SAFETY
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm -hwasan-generate-tags-with-calls -O0 %s -o - | FileCheck %s --check-prefix=NOSAFETY
+
+int main(int argc, char **argv) {
+  char buf[10];
+  volatile char *x = buf;
+  *x = 0;
+  return buf[0];
+  // NOSAFETY: __hwasan_generate_tag
+  // SAFETY-NOT: __hwasan_generate_tag
+}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,23 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
   explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  bool Recover = false,
+  const StackSafetyGlobalInfo *SSI = nullptr)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-15 Thread Florian Mayer via Phabricator via cfe-commits
fmayer added a comment.

Thanks!




Comment at: clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c:4
+
+int main(int argc, char **argv) {
+  char buf[10];

vitalybuka wrote:
> fmayer wrote:
> > vitalybuka wrote:
> > > this patch mostly change code under llvm/ so tests should be also there, 
> > > as IR tests
> > > 
> > > 
> > I don't have strong feelings, but clang/test/CodeGen/lifetime-sanitizer.c 
> > is a very similar test, so I think we should either move all of these to 
> > llvm/ or add the new ones here to clang/. What do you think?
> That lifetime tests how clang inserts lifetime markers. So it must be in 
> clang/ this is from https://reviews.llvm.org/D20759 which is clang only patch.
> Here the only change for clang is forwarded BuilderWrapper.getTargetTriple().
> I don't mind if you keep your tests here, but we also need something which 
> tests llvm without clang as you change llvm tranformation.
> Usually if contributor changes code in llvm/, expectation is that check-llvm 
> should discover regression. It's not always possible, but that's the goal and 
> easy to do with this patch.
Thanks for the explanation. Moved to llvm/.



Comment at: llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h:32
+  Triple TargetTriple = {});
   PreservedAnalyses run(Module , ModuleAnalysisManager );
   static bool isRequired() { return true; }

vitalybuka wrote:
> fmayer wrote:
> > vitalybuka wrote:
> > > Why not from M.getTargetTriple() ?
> > Mostly for consistency with the legacy pass. Either way is fine for me 
> > though, what do you prefer?
> I don't know if will cause any issues, but usually most passes get triple 
> from the module.
> I prefer we stay consistent with the rest of the code if possible.
> 
I'll leave it as is, for consistency within this file, as we need to do it this 
way for the new pass manager.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:444
+  const StackSafetyGlobalInfo *SSI = nullptr;
+  if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+SSI = (M);

vitalybuka wrote:
> we usually don't use {} for single line 
> also maybe good candidate for ?: operator
I think this would make a bit of a long tenary expression.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:390
+  void getAnalysisUsage(AnalysisUsage ) const override {
+if (shouldUseStackSafetyAnalysis(TargetTriple)) {
+  AU.addRequired();

vitalybuka wrote:
> fmayer wrote:
> > vitalybuka wrote:
> > > why we need to check TargetTriple for that?
> > Because we only need the stack safety analysis if we instrument the stack, 
> > which we do not do on x86_64 (see shouldInstrumentStack).
> I see, I forgot about this limitation.
> LGTM, but unconditional is fine as well, assuming we are going to have stack 
> instrumentation at some point?
I am not sure we will ever add it for non-LAM x86_64.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-15 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 358884.
fmayer marked 7 inline comments as done.
fmayer added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  llvm/test/CodeGen/hwasan-stack-safety-analysis.c

Index: llvm/test/CodeGen/hwasan-stack-safety-analysis.c
===
--- /dev/null
+++ llvm/test/CodeGen/hwasan-stack-safety-analysis.c
@@ -0,0 +1,15 @@
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -emit-llvm -mllvm -hwasan-use-stack-safety=true -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=SAFETY
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -emit-llvm -mllvm -hwasan-use-stack-safety=false -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=NOSAFETY
+
+// Default when optimizing, but not with O0.
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -emit-llvm -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=SAFETY
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -emit-llvm -mllvm -hwasan-generate-tags-with-calls -O0 %s -o - | FileCheck %s --check-prefix=NOSAFETY
+
+int main(int argc, char **argv) {
+  char buf[10];
+  volatile char *x = buf;
+  *x = 0;
+  return buf[0];
+  // NOSAFETY: __hwasan_generate_tag
+  // SAFETY-NOT: __hwasan_generate_tag
+}
Index: llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c
===
--- /dev/null
+++ llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c
@@ -0,0 +1,15 @@
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm -hwasan-use-stack-safety=true -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=SAFETY
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm -hwasan-use-stack-safety=false -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=NOSAFETY
+
+// Default when optimizing, but not with O0.
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s --check-prefix=SAFETY
+// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm -hwasan-generate-tags-with-calls -O0 %s -o - | FileCheck %s --check-prefix=NOSAFETY
+
+int main(int argc, char **argv) {
+  char buf[10];
+  volatile char *x = buf;
+  *x = 0;
+  return buf[0];
+  // NOSAFETY: __hwasan_generate_tag
+  // SAFETY-NOT: __hwasan_generate_tag
+}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,23 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple ,
+  bool DisableOptimization) {
+  auto StackSafety = ClUseStackSafety.getNumOccurrences()
+ ? ClUseStackSafety
+ : !DisableOptimization;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
   explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  bool Recover = false,
+  const StackSafetyGlobalInfo *SSI = nullptr)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-14 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

Other than missing llvm test is LGTM




Comment at: clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c:4
+
+int main(int argc, char **argv) {
+  char buf[10];

fmayer wrote:
> vitalybuka wrote:
> > this patch mostly change code under llvm/ so tests should be also there, as 
> > IR tests
> > 
> > 
> I don't have strong feelings, but clang/test/CodeGen/lifetime-sanitizer.c is 
> a very similar test, so I think we should either move all of these to llvm/ 
> or add the new ones here to clang/. What do you think?
That lifetime tests how clang inserts lifetime markers. So it must be in clang/ 
this is from https://reviews.llvm.org/D20759 which is clang only patch.
Here the only change for clang is forwarded BuilderWrapper.getTargetTriple().
I don't mind if you keep your tests here, but we also need something which 
tests llvm without clang as you change llvm tranformation.
Usually if contributor changes code in llvm/, expectation is that check-llvm 
should discover regression. It's not always possible, but that's the goal and 
easy to do with this patch.



Comment at: llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h:40
+  Triple TargetTriple;
+  bool IsOptNull;
 };

!IsOptNull -> Optimize
or
IsOptNull -> DisableOptimization




Comment at: llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h:32
+  Triple TargetTriple = {});
   PreservedAnalyses run(Module , ModuleAnalysisManager );
   static bool isRequired() { return true; }

fmayer wrote:
> vitalybuka wrote:
> > Why not from M.getTargetTriple() ?
> Mostly for consistency with the legacy pass. Either way is fine for me 
> though, what do you prefer?
I don't know if will cause any issues, but usually most passes get triple from 
the module.
I prefer we stay consistent with the rest of the code if possible.




Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:444
+  const StackSafetyGlobalInfo *SSI = nullptr;
+  if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+SSI = (M);

we usually don't use {} for single line 
also maybe good candidate for ?: operator



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:390
+  void getAnalysisUsage(AnalysisUsage ) const override {
+if (shouldUseStackSafetyAnalysis(TargetTriple)) {
+  AU.addRequired();

fmayer wrote:
> vitalybuka wrote:
> > why we need to check TargetTriple for that?
> Because we only need the stack safety analysis if we instrument the stack, 
> which we do not do on x86_64 (see shouldInstrumentStack).
I see, I forgot about this limitation.
LGTM, but unconditional is fine as well, assuming we are going to have stack 
instrumentation at some point?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-14 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 358579.
fmayer marked an inline comment as done.
fmayer added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,21 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple , bool IsOptNull) {
+  auto StackSafety =
+  ClUseStackSafety.getNumOccurrences() ? ClUseStackSafety : !IsOptNull;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
   explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  bool Recover = false,
+  const StackSafetyGlobalInfo *SSI = nullptr)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -225,6 +239,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -277,6 +293,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
@@ -347,8 +364,11 @@
   static char ID;
 
   explicit HWAddressSanitizerLegacyPass(bool CompileKernel = false,
-bool Recover = false)
-  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover) {
+bool Recover = false,
+Triple TargetTriple = {},
+bool IsOptNull = false)
+  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover),
+TargetTriple(TargetTriple) {
 initializeHWAddressSanitizerLegacyPassPass(
 *PassRegistry::getPassRegistry());
   }
@@ -356,11 +376,18 @@
   StringRef getPassName() const override { return "HWAddressSanitizer"; }
 
   bool doInitialization(Module ) override {
-HWASan = std::make_unique(M, CompileKernel, Recover);
+HWASan = std::make_unique(M, CompileKernel, Recover,
+  /*SSI=*/nullptr);
 return true;
   }
 
   bool runOnFunction(Function ) override {
+if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+  // We cannot call getAnalysis in doInitialization, that would cause a
+  // crash as the required analyses are not initialized yet.
+  HWASan->setSSI(
+  ().getResult());
+}
 return HWASan->sanitizeFunction(F);
   }
 
@@ -369,10 +396,18 @@
 return false;
   }
 
+  void getAnalysisUsage(AnalysisUsage ) const override {
+if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+  AU.addRequired();
+}
+  }
+
 private:
   std::unique_ptr HWASan;
   bool CompileKernel;
   bool Recover;
+  Triple TargetTriple;
+  bool IsOptNull;
 };
 
 } // end anonymous namespace
@@ -389,17 +424,27 @@
 false)
 
 FunctionPass *llvm::createHWAddressSanitizerLegacyPassPass(bool CompileKernel,
-   bool Recover) {
+

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-14 Thread Florian Mayer via Phabricator via cfe-commits
fmayer marked an inline comment as done and an inline comment as not done.
fmayer added a comment.

Addressed inline comments.




Comment at: clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c:4
+
+int main(int argc, char **argv) {
+  char buf[10];

vitalybuka wrote:
> this patch mostly change code under llvm/ so tests should be also there, as 
> IR tests
> 
> 
I don't have strong feelings, but clang/test/CodeGen/lifetime-sanitizer.c is a 
very similar test, so I think we should either move all of these to llvm/ or 
add the new ones here to clang/. What do you think?



Comment at: llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h:32
+  Triple TargetTriple = {});
   PreservedAnalyses run(Module , ModuleAnalysisManager );
   static bool isRequired() { return true; }

vitalybuka wrote:
> Why not from M.getTargetTriple() ?
Mostly for consistency with the legacy pass. Either way is fine for me though, 
what do you prefer?



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:390
+  void getAnalysisUsage(AnalysisUsage ) const override {
+if (shouldUseStackSafetyAnalysis(TargetTriple)) {
+  AU.addRequired();

vitalybuka wrote:
> why we need to check TargetTriple for that?
Because we only need the stack safety analysis if we instrument the stack, 
which we do not do on x86_64 (see shouldInstrumentStack).



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:1275
 bool HWAddressSanitizer::isInterestingAlloca(const AllocaInst ) {
+  // clang-format off
   return (AI.getAllocatedType()->isSized() &&

vitalybuka wrote:
> Instead of // clang-format off
> could you replace this with
> 
> ```
> # comment1
> if (...)
>   return false;
> 
> # comment2
> if (...)
>   return false;
> ...
> 
> ```
> 
> in a separate patch?
OK, will do in a followup (or see how I can get clang-tidy to do the right 
thing here).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-14 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 358577.
fmayer added a comment.

fixup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,21 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple , bool IsOptNull) {
+  auto StackSafety =
+  ClUseStackSafety.getNumOccurrences() ? ClUseStackSafety : !IsOptNull;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
   explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  bool Recover = false,
+  const StackSafetyGlobalInfo *SSI = nullptr)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -225,6 +239,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -277,6 +293,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
@@ -347,8 +364,11 @@
   static char ID;
 
   explicit HWAddressSanitizerLegacyPass(bool CompileKernel = false,
-bool Recover = false)
-  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover) {
+bool Recover = false,
+Triple TargetTriple = {},
+bool IsOptNull = false)
+  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover),
+TargetTriple(TargetTriple) {
 initializeHWAddressSanitizerLegacyPassPass(
 *PassRegistry::getPassRegistry());
   }
@@ -356,11 +376,18 @@
   StringRef getPassName() const override { return "HWAddressSanitizer"; }
 
   bool doInitialization(Module ) override {
-HWASan = std::make_unique(M, CompileKernel, Recover);
+HWASan = std::make_unique(M, CompileKernel, Recover,
+  /*SSI=*/nullptr);
 return true;
   }
 
   bool runOnFunction(Function ) override {
+if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+  // We cannot call getAnalysis in doInitialization, that would cause a
+  // crash as the required analyses are not initialized yet.
+  HWASan->setSSI(
+  ().getResult());
+}
 return HWASan->sanitizeFunction(F);
   }
 
@@ -369,10 +396,18 @@
 return false;
   }
 
+  void getAnalysisUsage(AnalysisUsage ) const override {
+if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+  AU.addRequired();
+}
+  }
+
 private:
   std::unique_ptr HWASan;
   bool CompileKernel;
   bool Recover;
+  Triple TargetTriple;
+  bool IsOptNull;
 };
 
 } // end anonymous namespace
@@ -389,17 +424,27 @@
 false)
 
 FunctionPass *llvm::createHWAddressSanitizerLegacyPassPass(bool CompileKernel,
-   bool Recover) {
+   bool Recover,

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-14 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 358567.
fmayer added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,21 @@
 #pragma GCC poison ClInstrumentWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple , bool IsOptNull) {
+  auto StackSafety =
+  ClUseStackSafety.getNumOccurrences() ? ClUseStackSafety : !IsOptNull;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
   explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  bool Recover = false,
+  const StackSafetyGlobalInfo *SSI = nullptr)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -225,6 +239,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -277,6 +293,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
@@ -347,8 +364,11 @@
   static char ID;
 
   explicit HWAddressSanitizerLegacyPass(bool CompileKernel = false,
-bool Recover = false)
-  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover) {
+bool Recover = false,
+Triple TargetTriple = {},
+bool IsOptNull = false)
+  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover),
+TargetTriple(TargetTriple) {
 initializeHWAddressSanitizerLegacyPassPass(
 *PassRegistry::getPassRegistry());
   }
@@ -356,11 +376,18 @@
   StringRef getPassName() const override { return "HWAddressSanitizer"; }
 
   bool doInitialization(Module ) override {
-HWASan = std::make_unique(M, CompileKernel, Recover);
+HWASan = std::make_unique(M, CompileKernel, Recover,
+  /*SSI=*/nullptr);
 return true;
   }
 
   bool runOnFunction(Function ) override {
+if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+  // We cannot call getAnalysis in doInitialization, that would cause a
+  // crash as the required analyses are not initialized yet.
+  HWASan->setSSI(
+  ().getResult());
+}
 return HWASan->sanitizeFunction(F);
   }
 
@@ -369,10 +396,18 @@
 return false;
   }
 
+  void getAnalysisUsage(AnalysisUsage ) const override {
+if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+  AU.addRequired();
+}
+  }
+
 private:
   std::unique_ptr HWASan;
   bool CompileKernel;
   bool Recover;
+  Triple TargetTriple;
+  bool IsOptNull;
 };
 
 } // end anonymous namespace
@@ -389,17 +424,27 @@
 false)
 
 FunctionPass *llvm::createHWAddressSanitizerLegacyPassPass(bool CompileKernel,
-   bool Recover) {
+   bool Recover,

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-14 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 358565.
fmayer added a comment.

fixup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -205,18 +211,26 @@
 }
 
 bool shouldInstrumentWithCalls(const Triple& TargetTriple) {
-  return ClInstrumentWithCalls && TargetTriple.getArch() == Triple::x86_64;
+  return ClInstrumentWithCalls || TargetTriple.getArch() == Triple::x86_64;
 // No one should use the option directly.
 #pragma GCC poison ClInstrumetnWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple , bool IsOptNull) {
+  auto StackSafety =
+  ClUseStackSafety.getNumOccurrences() ? ClUseStackSafety : !IsOptNull;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
   explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  bool Recover = false,
+  const StackSafetyGlobalInfo *SSI = nullptr)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -225,6 +239,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -277,6 +293,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
@@ -347,8 +364,11 @@
   static char ID;
 
   explicit HWAddressSanitizerLegacyPass(bool CompileKernel = false,
-bool Recover = false)
-  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover) {
+bool Recover = false,
+Triple TargetTriple = {},
+bool IsOptNull = false)
+  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover),
+TargetTriple(TargetTriple) {
 initializeHWAddressSanitizerLegacyPassPass(
 *PassRegistry::getPassRegistry());
   }
@@ -356,11 +376,18 @@
   StringRef getPassName() const override { return "HWAddressSanitizer"; }
 
   bool doInitialization(Module ) override {
-HWASan = std::make_unique(M, CompileKernel, Recover);
+HWASan = std::make_unique(M, CompileKernel, Recover,
+  /*SSI=*/nullptr);
 return true;
   }
 
   bool runOnFunction(Function ) override {
+if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+  // We cannot call getAnalysis in doInitialization, that would cause a
+  // crash as the required analyses are not initialized yet.
+  HWASan->setSSI(
+  ().getResult());
+}
 return HWASan->sanitizeFunction(F);
   }
 
@@ -369,10 +396,18 @@
 return false;
   }
 
+  void getAnalysisUsage(AnalysisUsage ) const override {
+if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+  AU.addRequired();
+}
+  }
+
 private:
   std::unique_ptr HWASan;
   bool CompileKernel;
   bool Recover;
+  Triple TargetTriple;
+  bool IsOptNull;
 };
 
 } // end anonymous namespace
@@ 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-14 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 358564.
fmayer marked 2 inline comments as done.
fmayer added a comment.

Address some comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,11 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"),
+ cl::Optional);
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -210,13 +216,21 @@
 #pragma GCC poison ClInstrumetnWithCalls
 }
 
+bool shouldUseStackSafetyAnalysis(const Triple , bool IsOptNull) {
+  auto StackSafety =
+  ClUseStackSafety.getNumOccurrences() ? ClUseStackSafety : !IsOptNull;
+  return shouldInstrumentStack(TargetTriple) && StackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
   explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  bool Recover = false,
+  const StackSafetyGlobalInfo *SSI = nullptr)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -225,6 +239,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -277,6 +293,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
@@ -347,8 +364,11 @@
   static char ID;
 
   explicit HWAddressSanitizerLegacyPass(bool CompileKernel = false,
-bool Recover = false)
-  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover) {
+bool Recover = false,
+Triple TargetTriple = {},
+bool IsOptNull = false)
+  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover),
+TargetTriple(TargetTriple) {
 initializeHWAddressSanitizerLegacyPassPass(
 *PassRegistry::getPassRegistry());
   }
@@ -356,11 +376,18 @@
   StringRef getPassName() const override { return "HWAddressSanitizer"; }
 
   bool doInitialization(Module ) override {
-HWASan = std::make_unique(M, CompileKernel, Recover);
+HWASan = std::make_unique(M, CompileKernel, Recover,
+  /*SSI=*/nullptr);
 return true;
   }
 
   bool runOnFunction(Function ) override {
+if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+  // We cannot call getAnalysis in doInitialization, that would cause a
+  // crash as the required analyses are not initialized yet.
+  HWASan->setSSI(
+  ().getResult());
+}
 return HWASan->sanitizeFunction(F);
   }
 
@@ -369,10 +396,18 @@
 return false;
   }
 
+  void getAnalysisUsage(AnalysisUsage ) const override {
+if (shouldUseStackSafetyAnalysis(TargetTriple, IsOptNull)) {
+  AU.addRequired();
+}
+  }
+
 private:
   std::unique_ptr HWASan;
   bool CompileKernel;
   bool Recover;
+  Triple TargetTriple;
+  bool IsOptNull;
 };
 
 } // end anonymous namespace
@@ -389,17 +424,27 @@
 false)
 
 FunctionPass *llvm::createHWAddressSanitizerLegacyPassPass(bool CompileKernel,
-   bool Recover) {
+

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-14 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added inline comments.



Comment at: clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c:4
+
+int main(int argc, char **argv) {
+  char buf[10];

this patch mostly change code under llvm/ so tests should be also there, as IR 
tests





Comment at: llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h:32
+  Triple TargetTriple = {});
   PreservedAnalyses run(Module , ModuleAnalysisManager );
   static bool isRequired() { return true; }

Why not from M.getTargetTriple() ?



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:200
 
+bool shouldUsePageAliases(const Triple ) {
+  return ClUsePageAliases && TargetTriple.getArch() == Triple::x86_64;

Could you please extract these function in a separate patch?
LLVM likes small close to NFC patches.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:390
+  void getAnalysisUsage(AnalysisUsage ) const override {
+if (shouldUseStackSafetyAnalysis(TargetTriple)) {
+  AU.addRequired();

why we need to check TargetTriple for that?



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:1275
 bool HWAddressSanitizer::isInterestingAlloca(const AllocaInst ) {
+  // clang-format off
   return (AI.getAllocatedType()->isSized() &&

Instead of // clang-format off
could you replace this with

```
# comment1
if (...)
  return false;

# comment2
if (...)
  return false;
...

```

in a separate patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-12 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis added a comment.

The analysis should not run at -O0.
Otherwise, LGTM.
What kind of code size improvement do you see?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-12 Thread Florian Mayer via Phabricator via cfe-commits
fmayer updated this revision to Diff 358043.
fmayer added a comment.

update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105703/new/

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,10 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"));
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -192,13 +197,31 @@
 
 namespace {
 
+bool shouldUsePageAliases(const Triple ) {
+  return ClUsePageAliases && TargetTriple.getArch() == Triple::x86_64;
+// No one should use the option directly.
+#pragma GCC poison ClUsePageAliases
+}
+
+bool shouldInstrumentStack(const Triple ) {
+  return shouldUsePageAliases(TargetTriple) ? false : ClInstrumentStack;
+// No one should use the option directly.
+#pragma GCC poison ClInstrumentStack
+}
+
+bool shouldUseStackSafetyAnalysis(const Triple ) {
+  return shouldInstrumentStack(TargetTriple) && ClUseStackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
   explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  bool Recover = false,
+  const StackSafetyGlobalInfo *SSI = nullptr)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -207,6 +230,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -259,6 +284,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
@@ -329,8 +355,10 @@
   static char ID;
 
   explicit HWAddressSanitizerLegacyPass(bool CompileKernel = false,
-bool Recover = false)
-  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover) {
+bool Recover = false,
+Triple TargetTriple = {})
+  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover),
+TargetTriple(TargetTriple) {
 initializeHWAddressSanitizerLegacyPassPass(
 *PassRegistry::getPassRegistry());
   }
@@ -338,11 +366,18 @@
   StringRef getPassName() const override { return "HWAddressSanitizer"; }
 
   bool doInitialization(Module ) override {
-HWASan = std::make_unique(M, CompileKernel, Recover);
+HWASan = std::make_unique(M, CompileKernel, Recover,
+  /*SSI=*/nullptr);
 return true;
   }
 
   bool runOnFunction(Function ) override {
+if (shouldUseStackSafetyAnalysis(TargetTriple)) {
+  // We cannot call getAnalysis in doInitialization, that would cause a
+  // crash as the required analyses are not initialized yet.
+  HWASan->setSSI(
+  ().getResult());
+}
 return HWASan->sanitizeFunction(F);
   }
 
@@ -351,10 +386,17 @@
 return false;
   }
 
+  void getAnalysisUsage(AnalysisUsage ) const override {
+if (shouldUseStackSafetyAnalysis(TargetTriple)) {
+  AU.addRequired();
+}
+  }
+
 private:
   std::unique_ptr HWASan;
   bool CompileKernel;
   bool Recover;
+  Triple TargetTriple;
 };
 
 } // end anonymous namespace
@@ -370,18 +412,25 @@
 "HWAddressSanitizer: detect memory bugs using tagged addressing.", false,
 false)
 
-FunctionPass 

[PATCH] D105703: [hwasan] Use stack safety analysis.

2021-07-12 Thread Florian Mayer via Phabricator via cfe-commits
fmayer created this revision.
Herald added subscribers: ormris, hiraditya.
fmayer updated this revision to Diff 357514.
fmayer added a comment.
fmayer edited the summary of this revision.
fmayer updated this revision to Diff 357544.
fmayer edited the summary of this revision.
fmayer updated this revision to Diff 357881.
fmayer edited the summary of this revision.
fmayer published this revision for review.
fmayer added a reviewer: eugenis.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Change commit message.


fmayer added a comment.

Formatting again.


fmayer added a comment.

Formatting.


This avoids unnecessary instrumentation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105703

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c
  clang/test/CodeGen/hwasan-stack-safety-analysis.c
  llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -109,6 +110,10 @@
cl::desc("instrument stack (allocas)"),
cl::Hidden, cl::init(true));
 
+static cl::opt
+ClUseStackSafety("hwasan-use-stack-safety", cl::Hidden, cl::init(true),
+ cl::Hidden, cl::desc("Use Stack Safety analysis results"));
+
 static cl::opt ClUARRetagToZero(
 "hwasan-uar-retag-to-zero",
 cl::desc("Clear alloca tags before returning from the function to allow "
@@ -192,13 +197,31 @@
 
 namespace {
 
+bool shouldUsePageAliases(const Triple ) {
+  return ClUsePageAliases && TargetTriple.getArch() == Triple::x86_64;
+// No one should use the option directly.
+#pragma GCC poison ClUsePageAliases
+}
+
+bool shouldInstrumentStack(const Triple ) {
+  return shouldUsePageAliases(TargetTriple) ? false : ClInstrumentStack;
+// No one should use the option directly.
+#pragma GCC poison ClInstrumentStack
+}
+
+bool shouldUseStackSafetyAnalysis(const Triple ) {
+  return shouldInstrumentStack(TargetTriple) && ClUseStackSafety;
+// No one should use the option directly.
+#pragma GCC poison ClUseStackSafety
+}
 /// An instrumentation pass implementing detection of addressability bugs
 /// using tagged pointers.
 class HWAddressSanitizer {
 public:
   explicit HWAddressSanitizer(Module , bool CompileKernel = false,
-  bool Recover = false)
-  : M(M) {
+  bool Recover = false,
+  const StackSafetyGlobalInfo *SSI = nullptr)
+  : M(M), SSI(SSI) {
 this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
 this->CompileKernel = ClEnableKhwasan.getNumOccurrences() > 0
   ? ClEnableKhwasan
@@ -207,6 +230,8 @@
 initializeModule();
   }
 
+  void setSSI(const StackSafetyGlobalInfo *S) { SSI = S; }
+
   bool sanitizeFunction(Function );
   void initializeModule();
   void createHwasanCtorComdat();
@@ -259,6 +284,7 @@
 private:
   LLVMContext *C;
   Module 
+  const StackSafetyGlobalInfo *SSI;
   Triple TargetTriple;
   FunctionCallee HWAsanMemmove, HWAsanMemcpy, HWAsanMemset;
   FunctionCallee HWAsanHandleVfork;
@@ -329,8 +355,10 @@
   static char ID;
 
   explicit HWAddressSanitizerLegacyPass(bool CompileKernel = false,
-bool Recover = false)
-  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover) {
+bool Recover = false,
+Triple TargetTriple = {})
+  : FunctionPass(ID), CompileKernel(CompileKernel), Recover(Recover),
+TargetTriple(TargetTriple) {
 initializeHWAddressSanitizerLegacyPassPass(
 *PassRegistry::getPassRegistry());
   }
@@ -338,11 +366,18 @@
   StringRef getPassName() const override { return "HWAddressSanitizer"; }
 
   bool doInitialization(Module ) override {
-HWASan = std::make_unique(M, CompileKernel, Recover);
+HWASan = std::make_unique(M, CompileKernel, Recover,
+  /*SSI=*/nullptr);
 return true;
   }
 
   bool runOnFunction(Function ) override {
+if (shouldUseStackSafetyAnalysis(TargetTriple)) {
+  // We cannot call getAnalysis in doInitialization, that would cause a
+  // crash as the required analyses are not initialized yet.
+  HWASan->setSSI(
+