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

Kirill Stoimenov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 18 09:28:14 PDT 2021


kstoimenov marked an inline comment as done.
kstoimenov added inline comments.


================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:157
   TypeSize TS = DL.getTypeAllocSize(AI.getAllocatedType());
-  unsigned PointerSize = DL.getMaxPointerSizeInBits();
+  unsigned PointerSize = DL.getPointerSizeInBits();
   // Fallback to empty range for alloca size.
----------------
vitalybuka wrote:
> kstoimenov wrote:
> > vitalybuka wrote:
> > > nikic wrote:
> > > > The pointer size depends on the address space, you probably want `DL.getPointerTypeSizeInBits(AI.getType())`.
> > > 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.
> > I can change it back if you think getPointerSizeInBits() is the right approach. 
> I'd prefer assert(  getAddressSpace() == 0
> and just getMaxPointerSizeInBits()
> 
> 
> with change at line ~825 to skip them
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. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111829



More information about the llvm-commits mailing list