Skip to content

Commit

Permalink
Widen EHScope::ClenupBitFields::FixupDepth to avoid overflowing it (P…
Browse files Browse the repository at this point in the history
…R23490)

It currently only takes 2048 gotos to overflow the FixupDepth bitfield,
causing silent miscompilation. Apparently some parser generators run into
this (see PR).

I don't know that that data structure is terribly size sensitive anyway,
and since there's no room to widen the bitfield, let's just use a separate
word in EHCatchScope for it.

Differential Revision: http://reviews.llvm.org/D21566

llvm-svn: 273434
  • Loading branch information
zmodem committed Jun 22, 2016
1 parent 0df3505 commit 9565cf5
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 9 deletions.
20 changes: 11 additions & 9 deletions clang/lib/CodeGen/CGCleanup.h
Expand Up @@ -86,11 +86,6 @@ class EHScope {
/// The amount of extra storage needed by the Cleanup.
/// Always a multiple of the scope-stack alignment.
unsigned CleanupSize : 12;

/// The number of fixups required by enclosing scopes (not including
/// this one). If this is the top cleanup scope, all the fixups
/// from this index onwards belong to this scope.
unsigned FixupDepth : 32 - 18 - NumCommonBits; // currently 12
};

class FilterBitFields {
Expand Down Expand Up @@ -188,6 +183,7 @@ class EHCatchScope : public EHScope {
EHScopeStack::stable_iterator enclosingEHScope)
: EHScope(Catch, enclosingEHScope) {
CatchBits.NumHandlers = numHandlers;
assert(CatchBits.NumHandlers == numHandlers && "NumHandlers overflow?");
}

unsigned getNumHandlers() const {
Expand Down Expand Up @@ -263,6 +259,11 @@ class LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) EHCleanupScope : public EHScope {
};
mutable struct ExtInfo *ExtInfo;

/// The number of fixups required by enclosing scopes (not including
/// this one). If this is the top cleanup scope, all the fixups
/// from this index onwards belong to this scope.
unsigned FixupDepth;

struct ExtInfo &getExtInfo() {
if (!ExtInfo) ExtInfo = new struct ExtInfo();
return *ExtInfo;
Expand All @@ -288,16 +289,16 @@ class LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) EHCleanupScope : public EHScope {
unsigned cleanupSize, unsigned fixupDepth,
EHScopeStack::stable_iterator enclosingNormal,
EHScopeStack::stable_iterator enclosingEH)
: EHScope(EHScope::Cleanup, enclosingEH), EnclosingNormal(enclosingNormal),
NormalBlock(nullptr), ActiveFlag(nullptr), ExtInfo(nullptr) {
: EHScope(EHScope::Cleanup, enclosingEH),
EnclosingNormal(enclosingNormal), NormalBlock(nullptr),
ActiveFlag(nullptr), ExtInfo(nullptr), FixupDepth(fixupDepth) {
CleanupBits.IsNormalCleanup = isNormal;
CleanupBits.IsEHCleanup = isEH;
CleanupBits.IsActive = isActive;
CleanupBits.IsLifetimeMarker = false;
CleanupBits.TestFlagInNormalCleanup = false;
CleanupBits.TestFlagInEHCleanup = false;
CleanupBits.CleanupSize = cleanupSize;
CleanupBits.FixupDepth = fixupDepth;

assert(CleanupBits.CleanupSize == cleanupSize && "cleanup size overflow");
}
Expand Down Expand Up @@ -343,7 +344,7 @@ class LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) EHCleanupScope : public EHScope {
return CleanupBits.TestFlagInEHCleanup;
}

unsigned getFixupDepth() const { return CleanupBits.FixupDepth; }
unsigned getFixupDepth() const { return FixupDepth; }
EHScopeStack::stable_iterator getEnclosingNormalCleanup() const {
return EnclosingNormal;
}
Expand Down Expand Up @@ -451,6 +452,7 @@ class EHFilterScope : public EHScope {
EHFilterScope(unsigned numFilters)
: EHScope(Filter, EHScopeStack::stable_end()) {
FilterBits.NumFilters = numFilters;
assert(FilterBits.NumFilters == numFilters && "NumFilters overflow");
}

static size_t getSizeForNumFilters(unsigned numFilters) {
Expand Down
26 changes: 26 additions & 0 deletions clang/test/CodeGen/fixup-depth-overflow.c
@@ -0,0 +1,26 @@
// RUN: %clang_cc1 -O1 -disable-llvm-optzns -emit-llvm -o - %s | FileCheck %s

#define M if (x) goto L1;
#define M10 M M M M M M M M M M
#define M100 M10 M10 M10 M10 M10 M10 M10 M10 M10 M10
#define M1000 M100 M100 M100 M100 M100 M100 M100 M100 M100 M100

void f(int x) {
int h;

// Many gotos to not-yet-emitted labels would cause EHScope's FixupDepth
// to overflow (PR23490).
M1000 M1000 M1000

if (x == 5) {
// This will cause us to emit a clean-up of the stack variable. If the
// FixupDepths are broken, fixups will erroneously get threaded through it.
int i;
}

L1:
return;
}

// CHECK-LABEL: define void @f
// CHECK-NOT: cleanup

0 comments on commit 9565cf5

Please sign in to comment.