[Sanitizers] Replaced getMaxPointerSizeInBits with getPointerSizeInBits, which was causing failures for 32bit x86.

[vitalybuka]

Can you add a test? maybe clone small piece of 64bit version.

[kstoimenov]

Added a test, which fails without my fix. I named it i386-bug-fix.ll, let me know if you prefer a better name.
The culprit is the 'target datalayout' statement. I didn't dig deeper, but it somehow makes the max pointer size to be 64.

[kstoimenov]

Need to actually add the FileCheck statement. Hold off your review.

[nikic]

llvm/lib/Analysis/StackSafetyAnalysis.cpp

157

The pointer size depends on the address space, you probably want DL.getPointerTypeSizeInBits(AI.getType()).

[vitalybuka]

llvm/lib/Analysis/StackSafetyAnalysis.cpp

157

getPointerSizeInBits() should be fine but we need to checks that address space is default. I am not sure this analysis is true for alternative address spaces and current users also ignore non default ones.

[kstoimenov]

Made the test run using FileCheck and changed one place to use DL.getPointerTypeSizeInBits(AI.getType()).

[kstoimenov]

llvm/lib/Analysis/StackSafetyAnalysis.cpp

157

I can change it back if you think getPointerSizeInBits() is the right approach.

756

I don't think we have access to AllocaInst here, right? I will keep it like this.

llvm/test/Analysis/StackSafetyAnalysis/i386-bug-fix.ll

2

Added the test instructions, PTAL.

[vitalybuka]

llvm/lib/Analysis/StackSafetyAnalysis.cpp

825

I guess here, if AddressSpace is not 0, then skip it for now.

[vitalybuka]

llvm/lib/Analysis/StackSafetyAnalysis.cpp

157

I'd prefer assert( getAddressSpace() == 0
and just getMaxPointerSizeInBits()

with change at line ~825 to skip them

[vitalybuka]

llvm/lib/Analysis/StackSafetyAnalysis.cpp

756

yes

[kstoimenov]

Added requires to the test. Maybe this will solve Debian and Win builts?

[vitalybuka]

LGTM with skip/assert
I guess in a separate patch we need to review how we handle accesses of different address spaces.

@nikic Thanks for pointing to address space issue.

[kstoimenov]

llvm/lib/Analysis/StackSafetyAnalysis.cpp

157

The issue is that this function is also called from another place and will fail if I add a check there too. I will keep the code as is.

llvm/lib/Analysis/StackSafetyAnalysis.cpp

raw_ostream &operator<<(raw_ostream &OS, const UseInfo<CalleeTy> &U) {
   return OS;
 }
 
 /// Calculate the allocation size of a given alloca. Returns empty range
 // in case of confution.
 ConstantRange getStaticAllocaSizeRange(const AllocaInst &AI) {
   const DataLayout &DL = AI.getModule()->getDataLayout();
   TypeSize TS = DL.getTypeAllocSize(AI.getAllocatedType());
-  unsigned PointerSize = DL.getMaxPointerSizeInBits();
+  unsigned PointerSize = DL.getPointerTypeSizeInBits(AI.getType());
   // Fallback to empty range for alloca size.
   ConstantRange R = ConstantRange::getEmpty(PointerSize);
   if (TS.isScalable())
     return R;
   APInt APSize(PointerSize, TS.getFixedSize(), true);
   if (APSize.isNonPositive())
     return R;
   if (AI.isArrayAllocation()) {
GVToSSI createGlobalStackSafetyInfo(
 
   for (auto &FnKV : Copy)
     for (auto &KV : FnKV.second.Params) {
       resolveAllCalls(KV.second, Index);
       if (KV.second.Range.isFullSet())
         KV.second.Calls.clear();
     }
 
-  uint32_t PointerSize = Copy.begin()
-                             ->first->getParent()
-                             ->getDataLayout()
-                             .getMaxPointerSizeInBits();
+  uint32_t PointerSize =
+      Copy.begin()->first->getParent()->getDataLayout().getPointerSizeInBits();
   StackSafetyDataFlowAnalysis<GlobalValue> SSDFA(PointerSize, std::move(Copy));
 
   for (auto &F : SSDFA.run()) {
     auto FI = F.second;
     auto &SrcF = Functions[F.first];
     for (auto &KV : FI.Allocas) {
       auto &A = KV.second;
       resolveAllCalls(A, Index);
if (!Info) {
     }
     Info.reset(new InfoTy{
         createGlobalStackSafetyInfo(std::move(Functions), Index), {}, {}});
 
     for (auto &FnKV : Info->Info) {
       for (auto &KV : FnKV.second.Allocas) {
         ++NumAllocaTotal;
         const AllocaInst *AI = KV.first;
         auto AIRange = getStaticAllocaSizeRange(*AI);
         if (AIRange.contains(KV.second.Range)) {
           Info->SafeAllocas.insert(AI);
           ++NumAllocaStackSafe;
         }
         for (const auto &A : KV.second.Accesses)
           Info->AccessIsUnsafe[A.first] |= !AIRange.contains(A.second);
       }
     }

llvm/test/Analysis/StackSafetyAnalysis/i386-bug-fix.ll

+; REQUIRES: i386-pc-linux-gnu
+
+; RUN: opt passes="print-stack-safety" -disable-output -mtriple=i386-pc-linux-gnu %s 2>&1 | FileCheck %s --check-prefixes=CHECK
+
+; CHECK:      @main
+; CHECK-NEXT:   args uses:
+; CHECK-NEXT:     argv[]: empty-set
+; CHECK-NEXT:   allocas uses:
+; CHECK-NEXT:     [4]: [0,4)
+; CHECK-NEXT:     [32]: full-set
+; CHECK-NEXT:   safe accesses:
+
+target datalayout = "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-f80:32-n8:16:32-S128"
+target triple = "i386-pc-linux-gnu"
+
+; Function Attrs: mustprogress norecurse sanitize_address uwtable
+define dso_local i32 @main(i32 %argc, i8** %argv)  {
+entry:
+  %0 = alloca i32, align 4
+  %1 = alloca i8, i64 32, align 32
+  %2 = ptrtoint i8* %1 to i32
+  store i32 %2, i32* %0, align 4
+  ret i32 0
+}