Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Computation of locals' offsets is wrong when stack size exceeds 2G #48911

Closed
nagisa opened this issue Mar 12, 2021 · 1 comment · Fixed by #84114 or #101840
Closed

Computation of locals' offsets is wrong when stack size exceeds 2G #48911

nagisa opened this issue Mar 12, 2021 · 1 comment · Fixed by #84114 or #101840
Labels
bugzilla Issues migrated from bugzilla llvm:codegen mc Machine (object) code

Comments

@nagisa
Copy link
Member

nagisa commented Mar 12, 2021

Bugzilla Link 49567
Version trunk
OS All
CC @topperc,@cuviper,@jdm,@RKSimon,@phoebewang,@rotateright,@oToToT

Extended Description

Given the following test case:

; RUN: llc -O0 -mtriple=x86_64 < %s
; RUN: llc -O0 -mtriple=i686 < %s

%large = type [2148000000 x i8]

define void @banana() unnamed_addr #0 {
  %1 = alloca %large, align 1
  %2 = alloca %large, align 1
  %3 = getelementptr inbounds %large, %large* %1, i64 0, i64 0
  store i8 42, i8* %3, align 1
  %4 = getelementptr inbounds %large, %large* %2, i64 0, i64 0
  store i8 43, i8* %4, align 1
  ret void
}

Will produce the following assembly:

movabsq	$4295999872, %rax               # imm = 0x1000FC180
subq	%rax, %rsp
.cfi_def_cfa_offset 1032584
movb	$42, -2146967424(%rsp)
movb	$43, -128(%rsp)
movabsq	$4295999872, %rax               # imm = 0x1000FC180
addq	%rax, %rsp
.cfi_def_cfa_offset 8
retq

You will notice that both the variables allocad in this function are considered to be below the already adjusted stack pointer, rather than above it, where they ought to be.

This looks like a basic signed 32-bit integer overflow somewhere in prologepilog:

# *** IR Dump Before Prologue/Epilogue Insertion & Frame Finalization (prologepilog) ***:
# Machine code for function banana: NoPHIs, TracksLiveness, NoVRegs, TiedOpsRewritten
Frame Objects:
  fi#0: size=2148000000, align=1, at location [SP+8]
  fi#1: size=2148000000, align=1, at location [SP+8]

bb.0 (%ir-block.0):
  MOV8mi %stack.0, 1, $noreg, 0, $noreg, 42 :: (store 1 into %ir.3)
  MOV8mi %stack.1, 1, $noreg, 0, $noreg, 43 :: (store 1 into %ir.4)
  RET 0

# *** IR Dump After Prologue/Epilogue Insertion & Frame Finalization (prologepilog) ***:
# Machine code for function banana: NoPHIs, TracksLiveness, NoVRegs, TiedOpsRewritten
Frame Objects:
  fi#0: size=2148000000, align=1, at location [SP-2148000000]
  fi#1: size=2148000000, align=1, at location [SP-4296000000]

bb.0 (%ir-block.0):
  $rax = frame-setup MOV64ri 4295999872
  $rsp = SUB64rr $rsp(tied-def 0), $rax, implicit-def dead $eflags
  CFI_INSTRUCTION def_cfa_offset 1032584
  MOV8mi $rsp, 1, $noreg, -2146967424, $noreg, 42 :: (store 1 into %ir.3)
  MOV8mi $rsp, 1, $noreg, -128, $noreg, 43 :: (store 1 into %ir.4)
  $rax = frame-destroy MOV64ri 4295999872
  $rsp = ADD64rr $rsp(tied-def 0), $rax, implicit-def dead $eflags
  CFI_INSTRUCTION def_cfa_offset 8
  RET 0

Worth noting that the the def_cfa_offset directive also overflowed in this reproducer here.

@nagisa
Copy link
Member Author

nagisa commented Mar 12, 2021

Reproducible as far back as LLVM 7, and as recently as of a2eca31

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
RKSimon pushed a commit that referenced this issue Mar 27, 2024
For very large stack frames, the offset from the stack pointer to a local can be more than 2^31 which overflows various `int` offsets in the frame lowering code.

This patch updates the frame lowering code to calculate the offsets as 64-bit values and resolves the overflows, resulting in the correct codegen for very large frames.

Fixes #48911
@EugeneZelenko EugeneZelenko added llvm:codegen mc Machine (object) code and removed backend:X86 labels Mar 27, 2024
@RKSimon RKSimon reopened this Mar 27, 2024
@arsenm arsenm closed this as completed in 0abb779 Aug 19, 2024
zmodem added a commit that referenced this issue Aug 21, 2024
…rames (#101840)"

This casuses assertion failures targeting 32-bit x86:

  lib/Target/X86/X86RegisterInfo.cpp:989:
  virtual bool llvm::X86RegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator, int, unsigned int, RegScavenger *) const:
  Assertion `(Is64Bit || FitsIn32Bits) && "Requesting 64-bit offset in 32-bit immediate!"' failed.

See comment on the PR.

> Fix 32-bit integer overflows in the X86 target frame layout when dealing
> with frames larger than 4gb. When this occurs, we'll scavenge a scratch
> register to be able to hold the correct stack offset for frame locals.
>
> This completes reapplying #84114.
>
> Fixes #48911
> Fixes #75944
> Fixes #87154

This reverts commit 0abb779.
cjdb pushed a commit to cjdb/llvm-project that referenced this issue Aug 23, 2024
…rames (llvm#101840)"

This casuses assertion failures targeting 32-bit x86:

  lib/Target/X86/X86RegisterInfo.cpp:989:
  virtual bool llvm::X86RegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator, int, unsigned int, RegScavenger *) const:
  Assertion `(Is64Bit || FitsIn32Bits) && "Requesting 64-bit offset in 32-bit immediate!"' failed.

See comment on the PR.

> Fix 32-bit integer overflows in the X86 target frame layout when dealing
> with frames larger than 4gb. When this occurs, we'll scavenge a scratch
> register to be able to hold the correct stack offset for frame locals.
>
> This completes reapplying llvm#84114.
>
> Fixes llvm#48911
> Fixes llvm#75944
> Fixes llvm#87154

This reverts commit 0abb779.
dmpolukhin pushed a commit to dmpolukhin/llvm-project that referenced this issue Sep 2, 2024
…rames (llvm#101840)"

This casuses assertion failures targeting 32-bit x86:

  lib/Target/X86/X86RegisterInfo.cpp:989:
  virtual bool llvm::X86RegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator, int, unsigned int, RegScavenger *) const:
  Assertion `(Is64Bit || FitsIn32Bits) && "Requesting 64-bit offset in 32-bit immediate!"' failed.

See comment on the PR.

> Fix 32-bit integer overflows in the X86 target frame layout when dealing
> with frames larger than 4gb. When this occurs, we'll scavenge a scratch
> register to be able to hold the correct stack offset for frame locals.
>
> This completes reapplying llvm#84114.
>
> Fixes llvm#48911
> Fixes llvm#75944
> Fixes llvm#87154

This reverts commit 0abb779.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla llvm:codegen mc Machine (object) code
Projects
None yet
3 participants