[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