Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Introduce CORINFO_EH_CLAUSE_SAMETRY flag for CoreRT ABI #8422

Merged
merged 1 commit into from
Dec 2, 2016
Merged

Introduce CORINFO_EH_CLAUSE_SAMETRY flag for CoreRT ABI #8422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/inc/corinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -911,10 +911,12 @@ enum CORINFO_ACCESS_FLAGS
// These are the flags set on an CORINFO_EH_CLAUSE
enum CORINFO_EH_CLAUSE_FLAGS
{
CORINFO_EH_CLAUSE_NONE = 0,
CORINFO_EH_CLAUSE_FILTER = 0x0001, // If this bit is on, then this EH entry is for a filter
CORINFO_EH_CLAUSE_FINALLY = 0x0002, // This clause is a finally clause
CORINFO_EH_CLAUSE_FAULT = 0x0004, // This clause is a fault clause
CORINFO_EH_CLAUSE_NONE = 0,
CORINFO_EH_CLAUSE_FILTER = 0x0001, // If this bit is on, then this EH entry is for a filter
CORINFO_EH_CLAUSE_FINALLY = 0x0002, // This clause is a finally clause
CORINFO_EH_CLAUSE_FAULT = 0x0004, // This clause is a fault clause
CORINFO_EH_CLAUSE_DUPLICATE = 0x0008, // Duplicated clause. This clause was duplicated to a funclet which was pulled out of line
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: CORINFO_EH_CLAUSE_DUPLICATE -- thanks for adding this!

CORINFO_EH_CLAUSE_SAMETRY = 0x0010, // This clause covers same try block as the previous one. (Used by CoreRT ABI.)
};

// This enumeration is passed to InternalThrow
Expand Down
33 changes: 25 additions & 8 deletions src/jit/codegencommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3319,6 +3319,8 @@ void CodeGen::genReportEH()
EHblkDsc* HBtab;
EHblkDsc* HBtabEnd;

bool isCoreRTABI = compiler->IsTargetAbi(CORINFO_CORERT_ABI);

unsigned EHCount = compiler->compHndBBtabCount;

#if FEATURE_EH_FUNCLETS
Expand All @@ -3328,7 +3330,7 @@ void CodeGen::genReportEH()
unsigned enclosingTryIndex;

// Duplicate clauses are not used by CoreRT ABI
if (!compiler->IsTargetAbi(CORINFO_CORERT_ABI))
if (!isCoreRTABI)
{
for (XTnum = 0; XTnum < compiler->compHndBBtabCount; XTnum++)
{
Expand All @@ -3347,7 +3349,7 @@ void CodeGen::genReportEH()
unsigned clonedFinallyCount = 0;

// Duplicate clauses are not used by CoreRT ABI
if (!compiler->IsTargetAbi(CORINFO_CORERT_ABI))
if (!isCoreRTABI)
{
// We don't keep track of how many cloned finally there are. So, go through and count.
// We do a quick pass first through the EH table to see if there are any try/finally
Expand Down Expand Up @@ -3429,6 +3431,23 @@ void CodeGen::genReportEH()

CORINFO_EH_CLAUSE_FLAGS flags = ToCORINFO_EH_CLAUSE_FLAGS(HBtab->ebdHandlerType);

if (isCoreRTABI && (XTnum > 0))
{
// For CoreRT, CORINFO_EH_CLAUSE_SAMETRY flag means that the current clause covers same
// try block as the previous one. The runtime cannot reliably infer this information from
// native code offsets because of different try blocks can have same offsets. Alternative
// solution to this problem would be inserting extra nops to ensure that different try
// blocks have different offsets.
if (EHblkDsc::ebdIsSameTry(HBtab, HBtab - 1))
{
// The SAMETRY bit should only be set on catch clauses. This is ensured in IL, where only 'catch' is
// allowed to be mutually-protect. E.g., the C# "try {} catch {} catch {} finally {}" actually exists in
// IL as "try { try {} catch {} catch {} } finally {}".
assert(HBtab->HasCatchHandler());
flags = (CORINFO_EH_CLAUSE_FLAGS)(flags | CORINFO_EH_CLAUSE_SAMETRY);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SAMETRY bit should only be set on catch clauses, not finally/fault/filter/filter-handler. You could probably assert that here. This is ensured in IL, where only 'catch' is allowed to be "mutually-protect". E.g., the C# try {} catch {} catch {} finally {} actually exists in IL as try { try {} catch {} catch {} } finally {}. Our EH normalization code and optimizations ensure that the two distinct try blocks here get different BasicBlocks, so ebdIsSameTry() won't ever return true.

However, I can't find anything that guarantees that the native code offsets for these two try regions don't end up identical, if the outer 'try' is otherwise empty (so we have empty head and tail BasicBlocks for it). I think the reason they end up with different native regions is because we have to generate code to call the finally block.

Maybe on the VM side (we could probably do it here as well), we should assert that if there are identical 'try' regions then the 2nd and subsequent ones have this new bit?

Copy link
Member Author

@jkotas jkotas Dec 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I have added assert for HasCatchHandler().

I will think about good asserts on the CoreRT side. The runtime won't actually see this bit as is. The whole table is compressed with variable length encoding, and there is no natural place for this bit in the compressed form. My current implementation is inserting empty clauses in the compressed form if it is necessary to explicitly separate runs with the same offset.

}
}

// Note that we reuse the CORINFO_EH_CLAUSE type, even though the names of
// the fields aren't accurate.

Expand Down Expand Up @@ -3634,9 +3653,7 @@ void CodeGen::genReportEH()
CORINFO_EH_CLAUSE_FLAGS flags = ToCORINFO_EH_CLAUSE_FLAGS(encTab->ebdHandlerType);

// Tell the VM this is an extra clause caused by moving funclets out of line.
// It seems weird this is from the CorExceptionFlag enum in corhdr.h,
// not the CORINFO_EH_CLAUSE_FLAGS enum in corinfo.h.
flags = (CORINFO_EH_CLAUSE_FLAGS)(flags | COR_ILEXCEPTION_CLAUSE_DUPLICATED);
flags = (CORINFO_EH_CLAUSE_FLAGS)(flags | CORINFO_EH_CLAUSE_DUPLICATE);

// Note that the JIT-EE interface reuses the CORINFO_EH_CLAUSE type, even though the names of
// the fields aren't really accurate. For example, we set "TryLength" to the offset of the
Expand Down Expand Up @@ -3703,9 +3720,9 @@ void CodeGen::genReportEH()

CORINFO_EH_CLAUSE clause;
clause.ClassToken = 0; // unused
clause.Flags = (CORINFO_EH_CLAUSE_FLAGS)(CORINFO_EH_CLAUSE_FINALLY | COR_ILEXCEPTION_CLAUSE_DUPLICATED);
clause.TryOffset = hndBeg;
clause.TryLength = hndBeg;
clause.Flags = (CORINFO_EH_CLAUSE_FLAGS)(CORINFO_EH_CLAUSE_FINALLY | CORINFO_EH_CLAUSE_DUPLICATE);
clause.TryOffset = hndBeg;
clause.TryLength = hndBeg;
clause.HandlerOffset = hndBeg;
clause.HandlerLength = hndEnd;

Expand Down
12 changes: 8 additions & 4 deletions src/jit/jiteh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2979,7 +2979,7 @@ void Compiler::dispOutgoingEHClause(unsigned num, const CORINFO_EH_CLAUSE& claus
// Note: the flags field is kind of weird. It should be compared for equality
// to determine the type of clause, even though it looks like a bitfield. In
// Particular, CORINFO_EH_CLAUSE_NONE is zero, so you can "&" to check it.
// You do need to mask off the bits, though, because COR_ILEXCEPTION_CLAUSE_DUPLICATED
// You do need to mask off the bits, though, because CORINFO_EH_CLAUSE_DUPLICATE
// is and'ed in.
const DWORD CORINFO_EH_CLAUSE_TYPE_MASK = 0x7;
switch (clause.Flags & CORINFO_EH_CLAUSE_TYPE_MASK)
Expand Down Expand Up @@ -3013,15 +3013,19 @@ void Compiler::dispOutgoingEHClause(unsigned num, const CORINFO_EH_CLAUSE& claus
}

if ((clause.TryOffset == clause.TryLength) && (clause.TryOffset == clause.HandlerOffset) &&
((clause.Flags & (COR_ILEXCEPTION_CLAUSE_DUPLICATED | COR_ILEXCEPTION_CLAUSE_FINALLY)) ==
(COR_ILEXCEPTION_CLAUSE_DUPLICATED | COR_ILEXCEPTION_CLAUSE_FINALLY)))
((clause.Flags & (CORINFO_EH_CLAUSE_DUPLICATE | CORINFO_EH_CLAUSE_FINALLY)) ==
(CORINFO_EH_CLAUSE_DUPLICATE | CORINFO_EH_CLAUSE_FINALLY)))
{
printf(" cloned finally");
}
else if (clause.Flags & COR_ILEXCEPTION_CLAUSE_DUPLICATED)
else if (clause.Flags & CORINFO_EH_CLAUSE_DUPLICATE)
{
printf(" duplicated");
}
else if (clause.Flags & CORINFO_EH_CLAUSE_SAMETRY)
{
printf(" same try");
}
printf("\n");
}

Expand Down