Skip to content

Commit cc612c2

Browse files
committed
[WebAssembly] Fix FastISel address calculation bug
Fixes PR47040, in which an assertion was improperly triggered during FastISel's address computation. The issue was that an `Address` set to be relative to the FrameIndex with offset zero was incorrectly considered to have an unset base. When the left hand side of an add set the Address to be 0 off the FrameIndex, the right side would not detect that the Address base had already been set and could try to set the Address to be relative to a register instead, triggering an assertion. This patch fixes the issue by explicitly tracking whether an `Address` has been set rather than interpreting an offset of zero to mean the `Address` has not been set. Differential Revision: https://reviews.llvm.org/D85581
1 parent 430db35 commit cc612c2

File tree

2 files changed

+30
-9
lines changed

2 files changed

+30
-9
lines changed

llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ class WebAssemblyFastISel final : public FastISel {
5858
int FI;
5959
} Base;
6060

61+
// Whether the base has been determined yet
62+
bool IsBaseSet = false;
63+
6164
int64_t Offset = 0;
6265

6366
const GlobalValue *GV = nullptr;
@@ -74,17 +77,19 @@ class WebAssemblyFastISel final : public FastISel {
7477
bool isFIBase() const { return Kind == FrameIndexBase; }
7578
void setReg(unsigned Reg) {
7679
assert(isRegBase() && "Invalid base register access!");
77-
assert(Base.Reg == 0 && "Overwriting non-zero register");
80+
assert(!IsBaseSet && "Base cannot be reset");
7881
Base.Reg = Reg;
82+
IsBaseSet = true;
7983
}
8084
unsigned getReg() const {
8185
assert(isRegBase() && "Invalid base register access!");
8286
return Base.Reg;
8387
}
8488
void setFI(unsigned FI) {
8589
assert(isFIBase() && "Invalid base frame index access!");
86-
assert(Base.FI == 0 && "Overwriting non-zero frame index");
90+
assert(!IsBaseSet && "Base cannot be reset");
8791
Base.FI = FI;
92+
IsBaseSet = true;
8893
}
8994
unsigned getFI() const {
9095
assert(isFIBase() && "Invalid base frame index access!");
@@ -98,13 +103,7 @@ class WebAssemblyFastISel final : public FastISel {
98103
int64_t getOffset() const { return Offset; }
99104
void setGlobalValue(const GlobalValue *G) { GV = G; }
100105
const GlobalValue *getGlobalValue() const { return GV; }
101-
bool isSet() const {
102-
if (isRegBase()) {
103-
return Base.Reg != 0;
104-
} else {
105-
return Base.FI != 0;
106-
}
107-
}
106+
bool isSet() const { return IsBaseSet; }
108107
};
109108

110109
/// Keep a pointer to the WebAssemblySubtarget around so that we can make the
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
; RUN: llc < %s -fast-isel -fast-isel-abort=1 -verify-machineinstrs
2+
3+
; Regression test for PR47040, in which an assertion was improperly
4+
; triggered during FastISel's address computation. The issue was that
5+
; an `Address` set to be relative to FrameIndex zero was incorrectly
6+
; considered to have an unset base. When the left hand side of an add
7+
; set the Address to have a FrameIndex base of 0, the right side would
8+
; not detect that the Address base had already been set and could try
9+
; to set the Address to be relative to a register instead, triggering
10+
; an assertion.
11+
12+
target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
13+
target triple = "wasm32-unknown-unknown"
14+
15+
define i32 @foo() {
16+
%stack_addr = alloca i32
17+
%stack_i = ptrtoint i32* %stack_addr to i32
18+
%added = add i32 %stack_i, undef
19+
%added_addr = inttoptr i32 %added to i32*
20+
%ret = load i32, i32* %added_addr
21+
ret i32 %ret
22+
}

0 commit comments

Comments
 (0)