[NFC][hwasan] Use `enum class` in `ShadowMapping` #109617

vitalybuka · 2024-09-23T07:26:17Z

No description provided.

Created using spr 1.3.4 [skip ci]

Created using spr 1.3.4

llvmbot · 2024-09-23T07:26:52Z

@llvm/pr-subscribers-llvm-transforms

Author: Vitaly Buka (vitalybuka)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/109617.diff

1 Files Affected:

(modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+30-43)

diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index be0ead40b573d8..2efb97c8759bc9 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -64,6 +64,7 @@
 #include "llvm/Transforms/Utils/MemoryTaggingSupport.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include "llvm/Transforms/Utils/PromoteMemToReg.h"
+#include <cstdint>
 #include <optional>
 #include <random>
 
@@ -83,8 +84,6 @@ const char kHwasanShadowMemoryDynamicAddress[] =
 static const size_t kNumberOfAccessSizes = 5;
 
 static const size_t kDefaultShadowScale = 4;
-static const uint64_t kDynamicShadowSentinel =
-    std::numeric_limits<uint64_t>::max();
 
 static const unsigned kShadowBaseAlignment = 32;
 
@@ -385,44 +384,44 @@ class HWAddressSanitizer {
   std::unique_ptr<RandomNumberGenerator> Rng;
 
   /// This struct defines the shadow mapping using the rule:
+  /// If `kFixed`, then
   ///   shadow = (mem >> Scale) + Offset.
-  /// If InGlobal is true, then
+  /// If `kGlobal`, then
+  ///   extern char* __hwasan_shadow_memory_dynamic_address;
+  ///   shadow = (mem >> Scale) + __hwasan_shadow_memory_dynamic_address
+  /// If `kIfunc`, then
   ///   extern char __hwasan_shadow[];
   ///   shadow = (mem >> Scale) + &__hwasan_shadow
-  /// If InTls is true, then
+  /// If `kTls`, then
   ///   extern char *__hwasan_tls;
   ///   shadow = (mem>>Scale) + align_up(__hwasan_shadow, kShadowBaseAlignment)
   ///
   /// If WithFrameRecord is true, then __hwasan_tls will be used to access the
   /// ring buffer for storing stack allocations on targets that support it.
   class ShadowMapping {
-    uint8_t Scale;
+    enum class OffsetKind {
+      kFixed = 0,
+      kGlobal,
+      kIfunc,
+      kTls,
+    };
+    OffsetKind Kind;
     uint64_t Offset;
-    bool InGlobal;
-    bool InTls;
+    uint8_t Scale;
     bool WithFrameRecord;
 
+    void SetFixed(uint64_t O) {
+      Kind = OffsetKind::kFixed;
+      Offset = O;
+    }
+
   public:
     void init(Triple &TargetTriple, bool InstrumentWithCalls);
     Align getObjectAlignment() const { return Align(1ULL << Scale); }
-    bool isInGlobal() const {
-      return !InGlobal && !InTls && Offset == kDynamicShadowSentinel;
-    }
-    bool isInifunc() const {
-      assert(!InGlobal || !InTls);
-      assert(!InGlobal || Offset == kDynamicShadowSentinel);
-      return InGlobal;
-    }
-    bool isInTls() const {
-      assert(!InTls || !InGlobal);
-      assert(!InTls || Offset == kDynamicShadowSentinel);
-      return InTls;
-    }
-    bool isFixed() const {
-      assert(Offset == kDynamicShadowSentinel || !InTls);
-      assert(Offset == kDynamicShadowSentinel || !InGlobal);
-      return Offset != kDynamicShadowSentinel;
-    }
+    bool isInGlobal() const { return Kind == OffsetKind::kGlobal; }
+    bool isInifunc() const { return Kind == OffsetKind::kIfunc; }
+    bool isInTls() const { return Kind == OffsetKind::kTls; }
+    bool isFixed() const { return Kind == OffsetKind::kFixed; }
     uint8_t scale() const { return Scale; };
     uint64_t offset() const {
       assert(isFixed());
@@ -1930,34 +1929,22 @@ void HWAddressSanitizer::ShadowMapping::init(Triple &TargetTriple,
   if (TargetTriple.isOSFuchsia()) {
     // Fuchsia is always PIE, which means that the beginning of the address
     // space is always available.
-    InGlobal = false;
-    InTls = false;
-    Offset = 0;
+    SetFixed(0);
     WithFrameRecord = true;
   } else if (ClMappingOffset.getNumOccurrences() > 0) {
-    InGlobal = false;
-    InTls = false;
-    Offset = ClMappingOffset;
+    SetFixed(ClMappingOffset);
     WithFrameRecord = false;
   } else if (ClEnableKhwasan || InstrumentWithCalls) {
-    InGlobal = false;
-    InTls = false;
-    Offset = 0;
+    SetFixed(0);
     WithFrameRecord = false;
   } else if (ClWithIfunc) {
-    InGlobal = true;
-    InTls = false;
-    Offset = kDynamicShadowSentinel;
+    Kind = OffsetKind::kIfunc;
     WithFrameRecord = false;
   } else if (ClWithTls) {
-    InGlobal = false;
-    InTls = true;
-    Offset = kDynamicShadowSentinel;
+    Kind = OffsetKind::kTls;
     WithFrameRecord = true;
   } else {
-    InGlobal = false;
-    InTls = false;
-    Offset = kDynamicShadowSentinel;
+    Kind = OffsetKind::kGlobal;
     WithFrameRecord = false;
   }
 }

llvmbot · 2024-09-23T07:26:54Z

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Vitaly Buka (vitalybuka)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/109617.diff

1 Files Affected:

(modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+30-43)

diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index be0ead40b573d8..2efb97c8759bc9 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -64,6 +64,7 @@
 #include "llvm/Transforms/Utils/MemoryTaggingSupport.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include "llvm/Transforms/Utils/PromoteMemToReg.h"
+#include <cstdint>
 #include <optional>
 #include <random>
 
@@ -83,8 +84,6 @@ const char kHwasanShadowMemoryDynamicAddress[] =
 static const size_t kNumberOfAccessSizes = 5;
 
 static const size_t kDefaultShadowScale = 4;
-static const uint64_t kDynamicShadowSentinel =
-    std::numeric_limits<uint64_t>::max();
 
 static const unsigned kShadowBaseAlignment = 32;
 
@@ -385,44 +384,44 @@ class HWAddressSanitizer {
   std::unique_ptr<RandomNumberGenerator> Rng;
 
   /// This struct defines the shadow mapping using the rule:
+  /// If `kFixed`, then
   ///   shadow = (mem >> Scale) + Offset.
-  /// If InGlobal is true, then
+  /// If `kGlobal`, then
+  ///   extern char* __hwasan_shadow_memory_dynamic_address;
+  ///   shadow = (mem >> Scale) + __hwasan_shadow_memory_dynamic_address
+  /// If `kIfunc`, then
   ///   extern char __hwasan_shadow[];
   ///   shadow = (mem >> Scale) + &__hwasan_shadow
-  /// If InTls is true, then
+  /// If `kTls`, then
   ///   extern char *__hwasan_tls;
   ///   shadow = (mem>>Scale) + align_up(__hwasan_shadow, kShadowBaseAlignment)
   ///
   /// If WithFrameRecord is true, then __hwasan_tls will be used to access the
   /// ring buffer for storing stack allocations on targets that support it.
   class ShadowMapping {
-    uint8_t Scale;
+    enum class OffsetKind {
+      kFixed = 0,
+      kGlobal,
+      kIfunc,
+      kTls,
+    };
+    OffsetKind Kind;
     uint64_t Offset;
-    bool InGlobal;
-    bool InTls;
+    uint8_t Scale;
     bool WithFrameRecord;
 
+    void SetFixed(uint64_t O) {
+      Kind = OffsetKind::kFixed;
+      Offset = O;
+    }
+
   public:
     void init(Triple &TargetTriple, bool InstrumentWithCalls);
     Align getObjectAlignment() const { return Align(1ULL << Scale); }
-    bool isInGlobal() const {
-      return !InGlobal && !InTls && Offset == kDynamicShadowSentinel;
-    }
-    bool isInifunc() const {
-      assert(!InGlobal || !InTls);
-      assert(!InGlobal || Offset == kDynamicShadowSentinel);
-      return InGlobal;
-    }
-    bool isInTls() const {
-      assert(!InTls || !InGlobal);
-      assert(!InTls || Offset == kDynamicShadowSentinel);
-      return InTls;
-    }
-    bool isFixed() const {
-      assert(Offset == kDynamicShadowSentinel || !InTls);
-      assert(Offset == kDynamicShadowSentinel || !InGlobal);
-      return Offset != kDynamicShadowSentinel;
-    }
+    bool isInGlobal() const { return Kind == OffsetKind::kGlobal; }
+    bool isInifunc() const { return Kind == OffsetKind::kIfunc; }
+    bool isInTls() const { return Kind == OffsetKind::kTls; }
+    bool isFixed() const { return Kind == OffsetKind::kFixed; }
     uint8_t scale() const { return Scale; };
     uint64_t offset() const {
       assert(isFixed());
@@ -1930,34 +1929,22 @@ void HWAddressSanitizer::ShadowMapping::init(Triple &TargetTriple,
   if (TargetTriple.isOSFuchsia()) {
     // Fuchsia is always PIE, which means that the beginning of the address
     // space is always available.
-    InGlobal = false;
-    InTls = false;
-    Offset = 0;
+    SetFixed(0);
     WithFrameRecord = true;
   } else if (ClMappingOffset.getNumOccurrences() > 0) {
-    InGlobal = false;
-    InTls = false;
-    Offset = ClMappingOffset;
+    SetFixed(ClMappingOffset);
     WithFrameRecord = false;
   } else if (ClEnableKhwasan || InstrumentWithCalls) {
-    InGlobal = false;
-    InTls = false;
-    Offset = 0;
+    SetFixed(0);
     WithFrameRecord = false;
   } else if (ClWithIfunc) {
-    InGlobal = true;
-    InTls = false;
-    Offset = kDynamicShadowSentinel;
+    Kind = OffsetKind::kIfunc;
     WithFrameRecord = false;
   } else if (ClWithTls) {
-    InGlobal = false;
-    InTls = true;
-    Offset = kDynamicShadowSentinel;
+    Kind = OffsetKind::kTls;
     WithFrameRecord = true;
   } else {
-    InGlobal = false;
-    InTls = false;
-    Offset = kDynamicShadowSentinel;
+    Kind = OffsetKind::kGlobal;
     WithFrameRecord = false;
   }
 }

Created using spr 1.3.4 [skip ci]

Created using spr 1.3.4

vitalybuka added 2 commits September 23, 2024 00:26

[𝘀𝗽𝗿] changes to main this commit is based on

164b887

Created using spr 1.3.4 [skip ci]

[𝘀𝗽𝗿] initial version

fe901d3

Created using spr 1.3.4

llvmbot added compiler-rt:sanitizer llvm:transforms labels Sep 23, 2024

vitalybuka requested a review from fmayer September 23, 2024 07:27

vitalybuka mentioned this pull request Sep 23, 2024

[hwasan] Allow stack traces even when fixed shadow is used #109344

Closed

fmayer approved these changes Sep 23, 2024

View reviewed changes

vitalybuka added 2 commits September 23, 2024 15:44

[𝘀𝗽𝗿] changes introduced through rebase

ea77831

Created using spr 1.3.4 [skip ci]

rebase

95f7cd1

Created using spr 1.3.4

vitalybuka changed the base branch from users/vitalybuka/spr/main.nfchwasan-use-enum-class-in-shadowmapping to main September 23, 2024 22:50

noinclude

3713411

Created using spr 1.3.4

vitalybuka merged commit 8dbb739 into main Sep 23, 2024
5 of 6 checks passed

vitalybuka deleted the users/vitalybuka/spr/nfchwasan-use-enum-class-in-shadowmapping branch September 23, 2024 22:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[NFC][hwasan] Use `enum class` in `ShadowMapping` #109617

[NFC][hwasan] Use `enum class` in `ShadowMapping` #109617

Uh oh!

vitalybuka commented Sep 23, 2024

Uh oh!

llvmbot commented Sep 23, 2024

Uh oh!

llvmbot commented Sep 23, 2024

Uh oh!

Uh oh!

Uh oh!

[NFC][hwasan] Use enum class in ShadowMapping #109617

[NFC][hwasan] Use enum class in ShadowMapping #109617

Uh oh!

Conversation

vitalybuka commented Sep 23, 2024

Uh oh!

llvmbot commented Sep 23, 2024

Uh oh!

llvmbot commented Sep 23, 2024

Uh oh!

Uh oh!

Uh oh!

[NFC][hwasan] Use `enum class` in `ShadowMapping` #109617

[NFC][hwasan] Use `enum class` in `ShadowMapping` #109617