From 3ac17efee8234daf31ab3275eaf02ef175deb467 Mon Sep 17 00:00:00 2001
From: Kostya Kortchinsky <kostyak@google.com>
Date: Fri, 8 Dec 2017 16:36:37 +0000
Subject: [scudo] Minor code generation improvement

Summary:
It looks like clang was generating somewhat weird assembly with the current
code. `FromPrimary`, even though `const`,  was replaced every time with the code
generated for `size <= SizeClassMap::kMaxSize` instead of using a variable or
register, and `FromPrimary` didn't induce `ClassId != 0` for the compiler, so a
dead branch was generated for `getActuallyAllocatedSize(Ptr, ClassId)` since
it's never called for `ClassId = 0` (Secondary backed allocations) [this one
was more wishful thinking on my side than anything else].

I rearranged the code bit so that the generated assembly is less clunky.

Also changed 2 whitespace inconsistencies that were bothering me.

Reviewers: alekseyshl, flowerhack

Reviewed By: flowerhack

Subscribers: llvm-commits, #sanitizers

Differential Revision: https://reviews.llvm.org/D40976

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@320160 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/scudo/scudo_allocator.cpp | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

(limited to 'lib/scudo/scudo_allocator.cpp')

diff --git a/lib/scudo/scudo_allocator.cpp b/lib/scudo/scudo_allocator.cpp
index 4f2243e50..6383c6819 100644
--- a/lib/scudo/scudo_allocator.cpp
+++ b/lib/scudo/scudo_allocator.cpp
@@ -370,18 +370,15 @@ struct ScudoAllocator {
       return FailureHandler::OnBadRequest();
 
     if (CheckRssLimit && UNLIKELY(isRssLimitExceeded()))
-        return FailureHandler::OnOOM();
+      return FailureHandler::OnOOM();
 
     // Primary and Secondary backed allocations have a different treatment. We
     // deal with alignment requirements of Primary serviced allocations here,
     // but the Secondary will take care of its own alignment needs.
-    const bool FromPrimary =
-        PrimaryAllocator::CanAllocate(AlignedSize, MinAlignment);
-
     void *Ptr;
     u8 ClassId;
     uptr AllocSize;
-    if (FromPrimary) {
+    if (PrimaryAllocator::CanAllocate(AlignedSize, MinAlignment)) {
       AllocSize = AlignedSize;
       ClassId = SizeClassMap::ClassID(AllocSize);
       ScudoTSD *TSD = getTSDAndLock();
@@ -396,7 +393,7 @@ struct ScudoAllocator {
       return FailureHandler::OnOOM();
 
     // If requested, we will zero out the entire contents of the returned chunk.
-    if ((ForceZeroContents || ZeroContents) && FromPrimary)
+    if ((ForceZeroContents || ZeroContents) && ClassId)
       memset(Ptr, 0, BackendAllocator.getActuallyAllocatedSize(Ptr, ClassId));
 
     UnpackedHeader Header = {};
@@ -406,23 +403,23 @@ struct ScudoAllocator {
       // Since the Secondary takes care of alignment, a non-aligned pointer
       // means it is from the Primary. It is also the only case where the offset
       // field of the header would be non-zero.
-      CHECK(FromPrimary);
+      CHECK(ClassId);
       UserBeg = RoundUpTo(UserBeg, Alignment);
       uptr Offset = UserBeg - AlignedChunkHeaderSize - BackendPtr;
       Header.Offset = Offset >> MinAlignmentLog;
     }
     CHECK_LE(UserBeg + Size, BackendPtr + AllocSize);
-    Header.ClassId = ClassId;
     Header.State = ChunkAllocated;
     Header.AllocType = Type;
-    if (FromPrimary) {
+    if (ClassId) {
+      Header.ClassId = ClassId;
       Header.SizeOrUnusedBytes = Size;
     } else {
       // The secondary fits the allocations to a page, so the amount of unused
       // bytes is the difference between the end of the user allocation and the
       // next page boundary.
-      uptr PageSize = GetPageSizeCached();
-      uptr TrailingBytes = (UserBeg + Size) & (PageSize - 1);
+      const uptr PageSize = GetPageSizeCached();
+      const uptr TrailingBytes = (UserBeg + Size) & (PageSize - 1);
       if (TrailingBytes)
         Header.SizeOrUnusedBytes = PageSize - TrailingBytes;
     }
-- 
cgit v1.2.3