This is an archive of the discontinued LLVM Phabricator instance.

SeparateConstOffsetFromGEP: Fix stack overflow in unreachable code
ClosedPublic

Authored by arsenm on Sep 13 2021, 7:44 PM.

Download Raw Diff

Details

Reviewers

jingyue
lebedev.ri
jdoerfert
aqjune
jroelofs
tra
spatel

Summary

ConstantOffsetExtractor::Find was infinitely recursing on the add
referencing itself.

Diff Detail

Event Timeline

arsenm created this revision.Sep 13 2021, 7:44 PM

Herald added a subscriber: hiraditya. · View Herald TranscriptSep 13 2021, 7:44 PM

arsenm requested review of this revision.Sep 13 2021, 7:44 PM

Herald added a project: Restricted Project. · View Herald TranscriptSep 13 2021, 7:44 PM

Herald added a subscriber: wdng. · View Herald Transcript

Harbormaster completed remote builds in B123779: Diff 372389.Sep 13 2021, 8:14 PM

LG. This is a rather idiomatic fix.

This revision is now accepted and ready to land.Sep 14 2021, 1:16 AM

tra accepted this revision.Sep 14 2021, 9:05 AM

88146230e1b21aa042da481e5fd702fab82408fc

Revision Contents

Path

Size

llvm/

lib/

Transforms/

Scalar/

SeparateConstOffsetFromGEP.cpp

3 lines

test/

Transforms/

SeparateConstOffsetFromGEP/

crash-in-unreachable-code.ll

14 lines

Diff 372389

llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp

	Show First 20 Lines • Show All 1,158 Lines • ▼ Show 20 Lines

	bool SeparateConstOffsetFromGEP::run(Function &F) {			bool SeparateConstOffsetFromGEP::run(Function &F) {
	if (DisableSeparateConstOffsetFromGEP)			if (DisableSeparateConstOffsetFromGEP)
	return false;			return false;

	DL = &F.getParent()->getDataLayout();			DL = &F.getParent()->getDataLayout();
	bool Changed = false;			bool Changed = false;
	for (BasicBlock &B : F) {			for (BasicBlock &B : F) {
				if (!DT->isReachableFromEntry(&B))
				continue;

	for (BasicBlock::iterator I = B.begin(), IE = B.end(); I != IE;)			for (BasicBlock::iterator I = B.begin(), IE = B.end(); I != IE;)
	if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(I++))			if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(I++))
	Changed \|= splitGEP(GEP);			Changed \|= splitGEP(GEP);
	// No need to split GEP ConstantExprs because all its indices are constant			// No need to split GEP ConstantExprs because all its indices are constant
	// already.			// already.
	}			}

	Changed \|= reuniteExts(F);			Changed \|= reuniteExts(F);
	▲ Show 20 Lines • Show All 211 Lines • Show Last 20 Lines

llvm/test/Transforms/SeparateConstOffsetFromGEP/crash-in-unreachable-code.ll

This file was added.

				; RUN: opt -mtriple=amdgcn-amd-amdhsa -separate-const-offset-from-gep %s

				@gv = external local_unnamed_addr addrspace(3) global [16 x i8], align 16

				; The add referencing itself is illegal, except it's in an unreachable block.
				define weak amdgpu_kernel void @foo() {
				entry:
				ret void

				for.body28.i: ; preds = %for.body28.i
				%arrayidx3389.i = getelementptr inbounds [16 x i8], [16 x i8] addrspace(3)* @gv, i32 0, i32 %inc38.7.i.1
				%inc38.7.i.1 = add nuw nsw i32 %inc38.7.i.1, 16
				br label %for.body28.i
				}