[llvm] 6c67ee0 - [MC] Fix PR45805: infinite recursion in assembler
Thomas Preud'homme via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 25 07:42:55 PDT 2020
Author: Thomas Preud'homme
Date: 2020-06-25T15:42:36+01:00
New Revision: 6c67ee0f58323fe39db84635ef8cea629c2214b4
URL: https://github.com/llvm/llvm-project/commit/6c67ee0f58323fe39db84635ef8cea629c2214b4
DIFF: https://github.com/llvm/llvm-project/commit/6c67ee0f58323fe39db84635ef8cea629c2214b4.diff
LOG: [MC] Fix PR45805: infinite recursion in assembler
Give up folding an expression if the fragment of one of the operands
would require laying out a fragment already being laid out. This
prevents hitting an infinite recursion when a fill size expression
refers to a later fragment since computing the offset of that fragment
would require laying out the fill fragment and thus computing its size
expression.
Reviewed By: echristo
Differential Revision: https://reviews.llvm.org/D79570
Added:
llvm/test/MC/AsmParser/layout-interdependency.s
Modified:
llvm/include/llvm/MC/MCAsmLayout.h
llvm/include/llvm/MC/MCFragment.h
llvm/lib/MC/MCAssembler.cpp
llvm/lib/MC/MCExpr.cpp
llvm/lib/MC/MCFragment.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCAsmLayout.h b/llvm/include/llvm/MC/MCAsmLayout.h
index 45ac96f0b81e..f95af210a28f 100644
--- a/llvm/include/llvm/MC/MCAsmLayout.h
+++ b/llvm/include/llvm/MC/MCAsmLayout.h
@@ -49,6 +49,10 @@ class MCAsmLayout {
/// Get the assembler object this is a layout for.
MCAssembler &getAssembler() const { return Assembler; }
+ /// \returns whether the offset of fragment \p F can be obtained via
+ /// getFragmentOffset.
+ bool canGetFragmentOffset(const MCFragment *F) const;
+
/// Invalidate the fragments starting with F because it has been
/// resized. The fragment's size should have already been updated, but
/// its bundle padding will be recomputed.
diff --git a/llvm/include/llvm/MC/MCFragment.h b/llvm/include/llvm/MC/MCFragment.h
index 4c8a895592ef..fb7166e82c09 100644
--- a/llvm/include/llvm/MC/MCFragment.h
+++ b/llvm/include/llvm/MC/MCFragment.h
@@ -65,6 +65,9 @@ class MCFragment : public ilist_node_with_parent<MCFragment, MCSection> {
FragmentType Kind;
+ /// Whether fragment is being laid out.
+ bool IsBeingLaidOut;
+
protected:
bool HasInstructions;
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 47b25c5a299b..c1a39cada7e2 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -397,6 +397,9 @@ void MCAsmLayout::layoutFragment(MCFragment *F) {
assert((!Prev || isFragmentValid(Prev)) &&
"Attempt to compute fragment before its predecessor!");
+ assert(!F->IsBeingLaidOut && "Already being laid out!");
+ F->IsBeingLaidOut = true;
+
++stats::FragmentLayouts;
// Compute fragment offset and size.
@@ -404,6 +407,7 @@ void MCAsmLayout::layoutFragment(MCFragment *F) {
F->Offset = Prev->Offset + getAssembler().computeFragmentSize(*this, *Prev);
else
F->Offset = 0;
+ F->IsBeingLaidOut = false;
LastValidFragment[F->getParent()] = F;
// If bundling is enabled and this fragment has instructions in it, it has to
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index 429531165585..fdb83aeef5eb 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -547,8 +547,10 @@ static void AttemptToFoldSymbolOffsetDifference(
if (!Asm->getWriter().isSymbolRefDifferenceFullyResolved(*Asm, A, B, InSet))
return;
- if (SA.getFragment() == SB.getFragment() && !SA.isVariable() &&
- !SA.isUnset() && !SB.isVariable() && !SB.isUnset()) {
+ MCFragment *FA = SA.getFragment();
+ MCFragment *FB = SB.getFragment();
+ if (FA == FB && !SA.isVariable() && !SA.isUnset() && !SB.isVariable() &&
+ !SB.isUnset()) {
Addend += (SA.getOffset() - SB.getOffset());
// Pointers to Thumb symbols need to have their low-bit set to allow
@@ -570,12 +572,17 @@ static void AttemptToFoldSymbolOffsetDifference(
if (!Layout)
return;
- const MCSection &SecA = *SA.getFragment()->getParent();
- const MCSection &SecB = *SB.getFragment()->getParent();
+ const MCSection &SecA = *FA->getParent();
+ const MCSection &SecB = *FB->getParent();
if ((&SecA != &SecB) && !Addrs)
return;
+ // One of the symbol involved is part of a fragment being laid out. Quit now
+ // to avoid a self loop.
+ if (!Layout->canGetFragmentOffset(FA) || !Layout->canGetFragmentOffset(FB))
+ return;
+
// Eagerly evaluate.
Addend += Layout->getSymbolOffset(A->getSymbol()) -
Layout->getSymbolOffset(B->getSymbol());
diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index 5252c3f14672..8e90e07a4dbf 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -48,6 +48,25 @@ bool MCAsmLayout::isFragmentValid(const MCFragment *F) const {
return F->getLayoutOrder() <= LastValid->getLayoutOrder();
}
+bool MCAsmLayout::canGetFragmentOffset(const MCFragment *F) const {
+ MCSection *Sec = F->getParent();
+ MCSection::iterator I;
+ if (MCFragment *LastValid = LastValidFragment[Sec]) {
+ // Fragment already valid, offset is available.
+ if (F->getLayoutOrder() <= LastValid->getLayoutOrder())
+ return true;
+ I = ++MCSection::iterator(LastValid);
+ } else
+ I = Sec->begin();
+
+ // A fragment ordered before F is currently being laid out.
+ const MCFragment *FirstInvalidFragment = &*I;
+ if (FirstInvalidFragment->IsBeingLaidOut)
+ return false;
+
+ return true;
+}
+
void MCAsmLayout::invalidateFragmentsFrom(MCFragment *F) {
// If this fragment wasn't already valid, we don't need to do anything.
if (!isFragmentValid(F))
@@ -235,7 +254,7 @@ void ilist_alloc_traits<MCFragment>::deleteNode(MCFragment *V) { V->destroy(); }
MCFragment::MCFragment(FragmentType Kind, bool HasInstructions,
MCSection *Parent)
: Parent(Parent), Atom(nullptr), Offset(~UINT64_C(0)), LayoutOrder(0),
- Kind(Kind), HasInstructions(HasInstructions) {
+ Kind(Kind), IsBeingLaidOut(false), HasInstructions(HasInstructions) {
if (Parent && !isa<MCDummyFragment>(*this))
Parent->getFragmentList().push_back(this);
}
diff --git a/llvm/test/MC/AsmParser/layout-interdependency.s b/llvm/test/MC/AsmParser/layout-interdependency.s
new file mode 100644
index 000000000000..6e275e00d9ec
--- /dev/null
+++ b/llvm/test/MC/AsmParser/layout-interdependency.s
@@ -0,0 +1,10 @@
+# RUN: not llvm-mc --filetype=obj %s -o /dev/null 2>&1 | FileCheck %s
+
+fct_end:
+
+# CHECK: layout-interdependency.s:[[#@LINE+1]]:7: error: expected assembly-time absolute expression
+.fill (data_start - fct_end), 1, 42
+# CHECK: layout-interdependency.s:[[#@LINE+1]]:7: error: expected assembly-time absolute expression
+.fill (fct_end - data_start), 1, 42
+
+data_start:
More information about the llvm-commits
mailing list