Append line number to variable name if line is available and in the same file as the function.

[vitalybuka]

update

[eugenis]

lib/Transforms/Instrumentation/AddressSanitizer.cpp

2270

while technically correct, "const unsigned" variables don't seem common in llvm. Remove const?

2272

I don't get this line at all. What's the meaning of the smallest of two line numbers?
Why do you store the line number in two places (inside the struct and as the second element of the pair?

[vitalybuka]

remove const

[vitalybuka]

lib/Transforms/Instrumentation/AddressSanitizer.cpp

2272

ASanStackVariableDescription is for allocas
But line information only available from lifteme intrinsics, and we can have several of them. I assume that smallest is where var was declared.

[eugenis]

lib/Transforms/Instrumentation/AddressSanitizer.cpp

2266

Why reset Pair.first?
Can it be replaced with assert(!Pair.first) ?

2272

OK. AILoc is a bad name, because it is not debugloc of AI, it's debugloc of the lifetime intrinstic.

Can this whole loop be moved below, after ASanStackVariableDescription objects are created, and then use Line field in the struct directly?

[vitalybuka]

update

[vitalybuka]

done

[vitalybuka]

lib/Transforms/Instrumentation/AddressSanitizer.cpp

2272

Can this whole loop be moved below, after ASanStackVariableDescription objects are created, and then use Line field in the struct directly?

It can't without changes in ComputeASanStackFrameLayout in ASanStackFrameLayout
It both sorts and builds stack description ASanStackVariableDescription and needs sizes and lines provided

[eugenis]

D25754 answers all my questions and makes this LGTM

[andrewford]

This broke android builds because it uses std::to_string instead of llvm::to_string. Would you mind changing it? https://reviews.llvm.org/D25754 adds another use of std::to_string. I would fix it but don't have commit access. Thanks!

include/llvm/Transforms/Utils/ASanStackFrameLayout.h

const char *Name;    // Name of the variable that will be displayed by asan
                        // if a stack-related bug is reported.
   uint64_t Size;       // Size of the variable in bytes.
   size_t LifetimeSize; // Size in bytes to use for lifetime analysis check.
                        // Will be rounded up to Granularity.
   size_t Alignment;    // Alignment of the variable (power of 2).
   AllocaInst *AI;      // The actual AllocaInst.
   size_t Offset;       // Offset from the beginning of the frame;
                        // set by ComputeASanStackFrameLayout.
+  unsigned Line;       // Line number.
 };
 
 // Output data struct for ComputeASanStackFrameLayout.
 struct ASanStackFrameLayout {
   size_t Granularity;
   // Frame description, see DescribeAddressIfStack in ASan runtime.
   SmallString<64> DescriptionString;
   size_t FrameAlignment;  // Alignment for the entire frame.

lib/Transforms/Instrumentation/AddressSanitizer.cpp

for (BasicBlock::iterator I(InsBefore); I != InsBeforeB->end(); ++I)
     if (auto *AI = dyn_cast<AllocaInst>(I))
       if (NonInstrumentedStaticAllocaVec.count(AI) > 0)
         AI->moveBefore(InsBefore);
 
   // If we have a call to llvm.localescape, keep it in the entry block.
   if (LocalEscapeCall) LocalEscapeCall->moveBefore(InsBefore);
 
   // Find static allocas with lifetime analysis.
-  DenseMap<const AllocaInst *, const ASanStackVariableDescription *>
+  DenseMap<const AllocaInst *,
+           std::pair<const ASanStackVariableDescription *, unsigned>>
       AllocaToSVDMap;
   for (const auto &APC : StaticAllocaPoisonCallVec) {
     assert(APC.InsBefore);
     assert(APC.AI);
     assert(ASan.isInterestingAlloca(*APC.AI));
     assert(APC.AI->isStaticAlloca());
 
     if (ClExperimentalPoisoning) {
-      AllocaToSVDMap[APC.AI] = nullptr;
+      auto &Pair = AllocaToSVDMap[APC.AI];
+      Pair.first = nullptr;
+      if (const DILocation *FnLoc = EntryDebugLocation.get()) {
+        if (const DILocation *AILoc = APC.InsBefore->getDebugLoc().get()) {
+          const unsigned Line = AILoc->getLine();
+          if (Line && AILoc->getFile() == FnLoc->getFile())
+            Pair.second = std::min(Pair.second ? Pair.second : Line, Line);
+        }
+      }
     } else {
       IRBuilder<> IRB(APC.InsBefore);
       poisonAlloca(APC.AI, APC.Size, IRB, APC.DoPoison);
     }
   }
 
   SmallVector<ASanStackVariableDescription, 16> SVD;
   SVD.reserve(AllocaVec.size());
   for (AllocaInst *AI : AllocaVec) {
-    size_t UseAfterScopePoisonSize =
-        AllocaToSVDMap.find(AI) != AllocaToSVDMap.end()
-            ? ASan.getAllocaSizeInBytes(*AI)
-            : 0;
     ASanStackVariableDescription D = {AI->getName().data(),
                                       ASan.getAllocaSizeInBytes(*AI),
-                                      UseAfterScopePoisonSize,
+                                      0,
                                       AI->getAlignment(),
                                       AI,
+                                      0,
                                       0};
+    auto It = AllocaToSVDMap.find(AI);
+    if (It != AllocaToSVDMap.end()) {
+      D.LifetimeSize = D.Size;
+      D.Line = It->second.second;
+    }
     SVD.push_back(D);
   }
   // Minimal header size (left redzone) is 4 pointers,
   // i.e. 32 bytes on 64-bit platforms and 16 bytes in 32-bit platforms.
   size_t MinHeaderSize = ASan.LongSize / 2;
   const ASanStackFrameLayout &L =
       ComputeASanStackFrameLayout(SVD, 1ULL << Mapping.Scale, MinHeaderSize);
 
void FunctionStackPoisoner::processStaticAllocas() {
   // As bytes we can use either the same or just red zones only.
   copyToShadow(ShadowAfterScope, ShadowAfterScope, IRB, ShadowBase);
 
   if (ClExperimentalPoisoning && !StaticAllocaPoisonCallVec.empty()) {
     // Complete AllocaToSVDMap
     for (const auto &Desc : SVD) {
       auto It = AllocaToSVDMap.find(Desc.AI);
       if (It != AllocaToSVDMap.end()) {
-        It->second = &Desc;
+        It->second.first = &Desc;
       }
     }
 
     const auto &ShadowInScope = GetShadowBytes(SVD, L);
 
     // Poison static allocas near lifetime intrinsics.
     for (const auto &APC : StaticAllocaPoisonCallVec) {
       // Must be already set.
-      assert(AllocaToSVDMap[APC.AI]);
-      const auto &Desc = *AllocaToSVDMap[APC.AI];
+      assert(AllocaToSVDMap[APC.AI].first);
+      const auto &Desc = *AllocaToSVDMap[APC.AI].first;
       assert(Desc.Offset % L.Granularity == 0);
       size_t Begin = Desc.Offset / L.Granularity;
       size_t End = Begin + (APC.Size + L.Granularity - 1) / L.Granularity;
 
       IRBuilder<> IRB(APC.InsBefore);
       copyToShadow(ShadowAfterScope,
                    APC.DoPoison ? ShadowAfterScope : ShadowInScope, Begin, End,
                    IRB, ShadowBase);

lib/Transforms/Utils/ASanStackFrameLayout.cpp

 //===-- ASanStackFrameLayout.cpp - helper for AddressSanitizer ------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Definition of ComputeASanStackFrameLayout (see ASanStackFrameLayout.h).
 //
 //===----------------------------------------------------------------------===//
 #include "llvm/Transforms/Utils/ASanStackFrameLayout.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/IR/DebugInfo.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 
 namespace llvm {
 
 // We sort the stack variables by alignment (largest first) to minimize
 // unnecessary large gaps due to alignment.
ComputeASanStackFrameLayout(SmallVectorImpl<ASanStackVariableDescription> &Vars,
   size_t Offset = std::max(std::max(MinHeaderSize, Granularity),
      Vars[0].Alignment);
   assert((Offset % Granularity) == 0);
   for (size_t i = 0; i < NumVars; i++) {
     bool IsLast = i == NumVars - 1;
     size_t Alignment = std::max(Granularity, Vars[i].Alignment);
     (void)Alignment;  // Used only in asserts.
     size_t Size = Vars[i].Size;
-    const char *Name = Vars[i].Name;
+    std::string Name = Vars[i].Name;
     assert((Alignment & (Alignment - 1)) == 0);
     assert(Layout.FrameAlignment >= Alignment);
     assert((Offset % Alignment) == 0);
     assert(Size > 0);
     assert(Vars[i].LifetimeSize <= Size);
-    StackDescription << " " << Offset << " " << Size << " " << strlen(Name)
+    if (Vars[i].Line) {
+      Name += ":";
+      Name += std::to_string(Vars[i].Line);
+    }
+    StackDescription << " " << Offset << " " << Size << " " << Name.size()
                      << " " << Name;
     size_t NextAlignment = IsLast ? Granularity
                    : std::max(Granularity, Vars[i + 1].Alignment);
     size_t SizeWithRedzone = VarAndRedzoneSize(Vars[i].Size, NextAlignment);
     Vars[i].Offset = Offset;
     Offset += SizeWithRedzone;
   }
   if (Offset % MinHeaderSize) {

test/Instrumentation/AddressSanitizer/stack_layout.ll

 ; Test the ASan's stack layout.
 ; More tests in tests/Transforms/Utils/ASanStackFrameLayoutTest.cpp
-; RUN: opt < %s -asan -asan-module -asan-stack-dynamic-alloca=0 -S \
-; RUN:     | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-STATIC
-; RUN: opt < %s -asan -asan-module -asan-stack-dynamic-alloca=1 -S \
-; RUN:     | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-DYNAMIC
+; RUN: opt < %s -asan -asan-module -asan-stack-dynamic-alloca=0 -asan-use-after-scope -S \
+; RUN:     | FileCheck %s --check-prefixes=CHECK,CHECK-STATIC
+; RUN: opt < %s -asan -asan-module -asan-stack-dynamic-alloca=1 -asan-use-after-scope -S \
+; RUN:     | FileCheck %s --check-prefixes=CHECK,CHECK-DYNAMIC
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 declare void @Use(i8*)
+declare void @llvm.lifetime.start(i64, i8* nocapture) nounwind
+declare void @llvm.lifetime.end(i64, i8* nocapture) nounwind
 
-; CHECK: private unnamed_addr constant{{.*}}3 32 10 3 XXX 64 20 3 YYY 128 30 3 ZZZ
-; CHECK: private unnamed_addr constant{{.*}}3 32 5 3 AAA 64 55 3 BBB 160 555 3 CCC
-; CHECK: private unnamed_addr constant{{.*}}3 256 128 3 CCC 448 128 3 BBB 608 128 3 AAA
+; CHECK: private unnamed_addr constant{{.*}}3 32 10 3 XXX 64 20 3 YYY 128 30 3 ZZZ\0
+; CHECK: private unnamed_addr constant{{.*}}3 32 5 3 AAA 64 55 3 BBB 160 555 3 CCC\0
+; CHECK: private unnamed_addr constant{{.*}}3 256 128 3 CCC 448 128 3 BBB 608 128 3 AAA\0
+; CHECK: private unnamed_addr constant{{.*}}2 32 4 3 AAA 48 4 5 BBB:7\0
 
 define void @Func1() sanitize_address {
 entry:
 ; CHECK-LABEL: Func1
 
 ; CHECK-STATIC: alloca [192 x i8]
 ; CHECK-DYNAMIC: alloca i8, i64 192
 
; CHECK: ret void
   %arr1.ptr = bitcast [128 x i8]* %AAA to i8*
   store volatile i8 0, i8* %arr1.ptr
   %arr2.ptr = bitcast [128 x i8]* %BBB to i8*
   store volatile i8 0, i8* %arr2.ptr
   %arr3.ptr = bitcast [128 x i8]* %CCC to i8*
   store volatile i8 0, i8* %arr3.ptr
   ret void
 }
+
+; Check that line numbers are attached to variable names if variable
+; in the same file as a function.
+define void @Func5() sanitize_address #0 !dbg !11 {
+  %AAA = alloca i32, align 4  ; File is not the same as !11
+  %BBB = alloca i32, align 4  ; File is the same as !11
+  %BBB.ptr = bitcast i32* %BBB to i8*
+  call void @llvm.lifetime.start(i64 4, i8* nonnull %BBB.ptr), !dbg !12
+  store volatile i32 5, i32* %BBB, align 4
+  %AAA.ptr = bitcast i32* %AAA to i8*
+  call void @llvm.lifetime.start(i64 4, i8* nonnull %AAA.ptr), !dbg !14
+  store volatile i32 3, i32* %AAA, align 4
+  call void @llvm.lifetime.end(i64 4, i8* nonnull %AAA.ptr), !dbg !17
+  call void @llvm.lifetime.end(i64 4, i8* nonnull %BBB.ptr), !dbg !18
+  ret void
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1)
+!1 = !DIFile(filename: "../file1.c", directory: "/")
+!11 = distinct !DISubprogram(name: "Func5", scope: !1, file: !1, line: 6, unit: !0)
+!12 = !DILocation(line: 7, column: 3, scope: !11)
+!18 = !DILocation(line: 10, column: 1, scope: !11)
+
+!21 = !DIFile(filename: "../file2.c", directory: "/")
+!6 = distinct !DISubprogram(name: "Func4", scope: !1, file: !21, line: 2, unit: !0)
+!15 = distinct !DILocation(line: 8, column: 3, scope: !11)
+!14 = !DILocation(line: 3, column: 3, scope: !6, inlinedAt: !15)
+!17 = !DILocation(line: 4, column: 1, scope: !6, inlinedAt: !15)

unittests/Transforms/Utils/ASanStackFrameLayoutTest.cpp

 
 // Use macro to preserve line information in EXPECT_EQ output.
 #define TEST_LAYOUT(V, Granularity, MinHeaderSize, ExpectedDescr,              \
                     ExpectedShadow, ExpectedShadowAfterScope)                  \
   {                                                                            \
     SmallVector<ASanStackVariableDescription, 10> Vars = V;                    \
     ASanStackFrameLayout L =                                                   \
         ComputeASanStackFrameLayout(Vars, Granularity, MinHeaderSize);         \
-    EXPECT_EQ(ExpectedDescr, L.DescriptionString);                             \
+    EXPECT_STREQ(ExpectedDescr, L.DescriptionString.c_str());                  \
     EXPECT_EQ(ExpectedShadow, ShadowBytesToString(GetShadowBytes(Vars, L)));   \
     EXPECT_EQ(ExpectedShadowAfterScope,                                        \
               ShadowBytesToString(GetShadowBytesAfterScope(Vars, L)));         \
   }
 
 TEST(ASanStackFrameLayout, Test) {
-#define VAR(name, size, lifetime, alignment)                                   \
+#define VAR(name, size, lifetime, alignment, line)                             \
   ASanStackVariableDescription name##size##_##alignment = {                    \
     #name #size "_" #alignment,                                                \
     size,                                                                      \
     lifetime,                                                                  \
     alignment,                                                                 \
     0,                                                                         \
-    0                                                                          \
+    0,                                                                         \
+    line,                                                                      \
   }
 
-  VAR(a, 1, 0, 1);
-  VAR(p, 1, 0, 32);
-  VAR(p, 1, 0, 256);
-  VAR(a, 2, 0, 1);
-  VAR(a, 3, 0, 1);
-  VAR(a, 4, 0, 1);
-  VAR(a, 7, 0, 1);
-  VAR(a, 8, 8, 1);
-  VAR(a, 9, 0, 1);
-  VAR(a, 16, 16, 1);
-  VAR(a, 41, 9, 1);
-  VAR(a, 105, 103, 1);
+  VAR(a, 1, 0, 1, 0);
+  VAR(p, 1, 0, 32, 15);
+  VAR(p, 1, 0, 256, 2700);
+  VAR(a, 2, 0, 1, 0);
+  VAR(a, 3, 0, 1, 0);
+  VAR(a, 4, 0, 1, 0);
+  VAR(a, 7, 0, 1, 0);
+  VAR(a, 8, 8, 1, 0);
+  VAR(a, 9, 0, 1, 0);
+  VAR(a, 16, 16, 1, 0);
+  VAR(a, 41, 9, 1, 7);
+  VAR(a, 105, 103, 1, 0);
 
   TEST_LAYOUT({a1_1}, 8, 16, "1 16 1 4 a1_1", "LL1R", "LL1R");
   TEST_LAYOUT({a1_1}, 64, 64, "1 64 1 4 a1_1", "L1", "L1");
-  TEST_LAYOUT({p1_32}, 8, 32, "1 32 1 5 p1_32", "LLLL1RRR", "LLLL1RRR");
-  TEST_LAYOUT({p1_32}, 8, 64, "1 64 1 5 p1_32", "LLLLLLLL1RRRRRRR",
+  TEST_LAYOUT({p1_32}, 8, 32, "1 32 1 8 p1_32:15", "LLLL1RRR", "LLLL1RRR");
+  TEST_LAYOUT({p1_32}, 8, 64, "1 64 1 8 p1_32:15", "LLLLLLLL1RRRRRRR",
               "LLLLLLLL1RRRRRRR");
 
   TEST_LAYOUT({a1_1}, 8, 32, "1 32 1 4 a1_1", "LLLL1RRR", "LLLL1RRR");
   TEST_LAYOUT({a2_1}, 8, 32, "1 32 2 4 a2_1", "LLLL2RRR", "LLLL2RRR");
   TEST_LAYOUT({a3_1}, 8, 32, "1 32 3 4 a3_1", "LLLL3RRR", "LLLL3RRR");
   TEST_LAYOUT({a4_1}, 8, 32, "1 32 4 4 a4_1", "LLLL4RRR", "LLLL4RRR");
   TEST_LAYOUT({a7_1}, 8, 32, "1 32 7 4 a7_1", "LLLL7RRR", "LLLL7RRR");
   TEST_LAYOUT({a8_1}, 8, 32, "1 32 8 4 a8_1", "LLLL0RRR", "LLLLSRRR");
   TEST_LAYOUT({a9_1}, 8, 32, "1 32 9 4 a9_1", "LLLL01RR", "LLLL01RR");
   TEST_LAYOUT({a16_1}, 8, 32, "1 32 16 5 a16_1", "LLLL00RR", "LLLLSSRR");
-  TEST_LAYOUT({p1_256}, 8, 32, "1 256 1 6 p1_256",
+  TEST_LAYOUT({p1_256}, 8, 32, "1 256 1 11 p1_256:2700",
               "LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL1RRR",
               "LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL1RRR");
-  TEST_LAYOUT({a41_1}, 8, 32, "1 32 41 5 a41_1", "LLLL000001RRRRRR",
+  TEST_LAYOUT({a41_1}, 8, 32, "1 32 41 7 a41_1:7", "LLLL000001RRRRRR",
               "LLLLSS0001RRRRRR");
   TEST_LAYOUT({a105_1}, 8, 32, "1 32 105 6 a105_1", "LLLL00000000000001RRRRRR",
               "LLLLSSSSSSSSSSSSS1RRRRRR");
 
   {
     SmallVector<ASanStackVariableDescription, 10> t = {a1_1, p1_256};
-    TEST_LAYOUT(t, 8, 32, "2 256 1 6 p1_256 272 1 4 a1_1",
+    TEST_LAYOUT(t, 8, 32, "2 256 1 11 p1_256:2700 272 1 4 a1_1",
                 "LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL1M1R",
                 "LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL1M1R");
   }
 
   {
     SmallVector<ASanStackVariableDescription, 10> t = {a1_1, a16_1, a41_1};
-    TEST_LAYOUT(t, 8, 32, "3 32 1 4 a1_1 48 16 5 a16_1 80 41 5 a41_1",
+    TEST_LAYOUT(t, 8, 32, "3 32 1 4 a1_1 48 16 5 a16_1 80 41 7 a41_1:7",
                 "LLLL1M00MM000001RRRR", "LLLL1MSSMMSS0001RRRR");
   }
 #undef VAR
 #undef TEST_LAYOUT
 }