Could a gatekeeper please review the attached patch to fix bug 735:
C++ exception handling introduces false catch-all ranges.
This is a long-standing bug in C++ exception handling that leads to
incorrect runtime behavior -- when there is a catch-all range and a
cleanup range in the same region, the cg could incorrectly mark the
cleanup range as having a catch-all handler as well. The bug is
present at all optimization levels, although aggressive inlining tends
to expose the bug more by creating more regions that meet the
preconditions.
The fundamental problem is that there is not enough information to
distinguish the catch-all range from the cleanup range once we are in
the cg. The fix introduces a special value to mark the catch-all in
wgen, propagates this information where necessary, and then handles
the special value in the cg with no ambiguity. The details are in the
attached msg.txt file.
I think that the test case in the bug report will fail on IA-64 for
the same reason, and a similar change will need to be made for the
IA-64 version of Create_INITO_For_Range_Table in the cg to complete
the fix for that target. If someone with access to IA-64 hardware can
provide that change, I will include it in the patch. Thanks,
-David Coakley / AMD Open Source Compiler Engineering
Fix bug 735: C++ exception handling introduces false catch-all regions.
The root cause of the problem is that the two cases of catch-all (no
type filter) and cleanup (landing pad but no handler, and therefore no
type) are not distinguished in the compiler's internal representation
of EH info. A "hack" used to tell the difference in the CG appears to
have incorrect assummptions as it is easy to generate a test case where
a cleanup is marked as a catch-all in the output range table, and will
thus fail at runtime when the C++ runtime treats the cleanup landing
pad as if it were a handler.
This fix uses the formerly unused INITV "value" of INITVKIND_ONE to
definitively represent the catch-all case.
- In wgen, rename "append_catch_all" to "append_cleanup". This routine
appends a potential cleanup, not a catch-all marker. Use the type
filter value of INITVKIND_ONE for real catch-all regions.
- In the cg, the detection of catch-all is simplified and no guesswork
is needed.
- Adjust code that writes or copies the internal EH info to expect
INITVKIND_ONE as a possible type filter value.
Index: osprey/be/cg/eh_region.cxx
===================================================================
--- osprey/be/cg/eh_region.cxx (revision 3512)
+++ osprey/be/cg/eh_region.cxx (working copy)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009 Advanced Micro Devices, Inc. All Rights Reserved.
+ * Copyright (C) 2009, 2011 Advanced Micro Devices, Inc. All Rights Reserved.
*/
/*
@@ -1859,34 +1859,18 @@
next_initv; next_initv=INITV_next (next_initv))
{
INITV_IDX action = New_INITV();
- int sym=0;
- if (INITV_kind(next_initv) != INITVKIND_ZERO)
+ // The special value INITVKIND_ONE represents a catch-all handler.
+ // A zero in the list means no handler, although there may still
+ // be a landing pad (cleanup).
+ int sym = 0;
+ bool catch_all = false;
+ if (INITV_kind(next_initv) == INITVKIND_ONE) {
+ FmtAssert (pad_label, ("Catch-all with no landing pad"));
+ catch_all = true;
+ }
+ else if (INITV_kind(next_initv) != INITVKIND_ZERO)
sym = TCON_uval(INITV_tc_val (next_initv));
- bool catch_all = false; // catch-all clause
- if (!sym)
- catch_all = (type_filter_map.find(sym) != type_filter_map.end());
- // if a region has eh specifications, we at least need:
- // 0 /* zero means zero */
- // 1 /* offset */
- // -filter /* eh spec typeinfo offset */
- // 0
- // But catch-all typeinfo is also zero. How to distinguish between
- // the first zero and a catch-all zero? The following hack is used:
- // if the next typeinfo is negative, then "zero means zero".
- if (catch_all && INITV_next (next_initv))
- {
- int next_sym = 0;
- INITV_IDX tmp_idx = INITV_next (next_initv);
- if (INITV_kind (tmp_idx) != INITVKIND_ZERO)
- next_sym = TCON_ival (INITV_tc_val (tmp_idx));
- if (next_sym < 0)
- catch_all = false;
- }
-
- // If there is no landing pad for this region, there should not be
- // a catch-all clause for it.
- if (catch_all && !pad_label) catch_all = false;
// action field
// Check if we have any action for this eh-region, if not, emit 0
// for action start marker.
Index: osprey/common/com/ir_bcom.cxx
===================================================================
--- osprey/common/com/ir_bcom.cxx (revision 3512)
+++ osprey/common/com/ir_bcom.cxx (working copy)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009 Advanced Micro Devices, Inc. All Rights Reserved.
+ * Copyright (C) 2009, 2011 Advanced Micro Devices, Inc. All Rights Reserved.
*/
/*
@@ -495,7 +495,7 @@
INITV_IDX types = INITV_next (INITV_blk (INITO_val (WN_ereg_supp (node))));
for (; types; types = INITV_next (types))
{
- if (INITV_kind (types) == INITVKIND_ZERO)
+ if (INITV_kind (types) != INITVKIND_VAL)
continue;
int index = TCON_uval (INITV_tc_val (types));
if (index <= 0) continue;
Index: osprey/wgen/wgen_stmt.cxx
===================================================================
--- osprey/wgen/wgen_stmt.cxx (revision 3512)
+++ osprey/wgen/wgen_stmt.cxx (working copy)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009-2010 Advanced Micro Devices, Inc. All Rights Reserved.
+ * Copyright (C) 2009-2011 Advanced Micro Devices, Inc. All Rights Reserved.
*/
/*
@@ -3080,8 +3080,13 @@
gs_t type = gs_handler_type(h);
ST_IDX st = 0;
- if (type) st = ST_st_idx (Get_ST (Get_typeinfo_var(type)));
- INITV_Set_VAL (Initv_Table[type_st], Enter_tcon (Host_To_Targ (MTYPE_U4, st)), 1);
+ if (type)
+ {
+ st = ST_st_idx (Get_ST (Get_typeinfo_var(type)));
+ INITV_Set_VAL (Initv_Table[type_st], Enter_tcon (Host_To_Targ (MTYPE_U4, st)), 1);
+ }
+ else // catch-all handler
+ INITV_Set_ONE (Initv_Table[type_st], MTYPE_U4, 1);
if (prev_type_st) Set_INITV_next (prev_type_st, type_st);
else start = type_st;
@@ -3178,17 +3183,18 @@
else iv = eh_filter;
}
+// Zero represents no handler, but possibly an associated landing pad.
static void
-append_catch_all (INITV_IDX& iv)
+append_cleanup (INITV_IDX& iv)
{
INITV_IDX tmp = iv;
while (tmp && INITV_next (tmp))
tmp = INITV_next (tmp);
- INITV_IDX catch_all = New_INITV();
- INITV_Set_VAL (Initv_Table[catch_all], Enter_tcon (Host_To_Targ (MTYPE_U4, 0)), 1);
- if (tmp) Set_INITV_next (tmp, catch_all);
- else iv = catch_all;
+ INITV_IDX cleanup = New_INITV();
+ INITV_Set_VAL (Initv_Table[cleanup], Enter_tcon (Host_To_Targ (MTYPE_U4, 0)), 1);
+ if (tmp) Set_INITV_next (tmp, cleanup);
+ else iv = cleanup;
}
// current: D1 D2 D3, prev: D1 D2 => emit D3 for current, goto prev
@@ -3285,11 +3291,11 @@
Set_PU_needs_manual_unwinding (Get_Current_PU());
// the following 2 calls can change 'iv'.
// NOTE: CG expects a zero before eh-spec filter
- bool catch_all_appended = false;
+ bool cleanup_appended = false;
if (PU_needs_manual_unwinding (Get_Current_PU()))
{
- append_catch_all (iv);
- catch_all_appended = true;
+ append_cleanup (iv);
+ cleanup_appended = true;
}
if (processing_handler)
{
@@ -3297,15 +3303,15 @@
FmtAssert (eh_spec, ("Invalid eh_spec inside handler"));
if (!eh_spec->empty())
{
- if (!catch_all_appended)
- append_catch_all (iv);
+ if (!cleanup_appended)
+ append_cleanup (iv);
append_eh_filter (iv);
}
}
else if (!eh_spec_vector.empty())
{
- if (!catch_all_appended)
- append_catch_all (iv);
+ if (!cleanup_appended)
+ append_cleanup (iv);
append_eh_filter (iv);
}
if (!iv)
Index: osprey/ipa/main/optimize/ipo_main.cxx
===================================================================
--- osprey/ipa/main/optimize/ipo_main.cxx (revision 3512)
+++ osprey/ipa/main/optimize/ipo_main.cxx (working copy)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2008-2009 Advanced Micro Devices, Inc. All Rights Reserved.
+ * Copyright (C) 2008-2009, 2011 Advanced Micro Devices, Inc. All Rights Reserved.
*/
/*
@@ -451,7 +451,7 @@
INITV_IDX types = INITV_next (INITV_blk (blk));
for (; types; types = INITV_next (types))
{
- if (INITV_kind (types) == INITVKIND_ZERO)
+ if (INITV_kind (types) != INITVKIND_VAL)
continue;
int index = TCON_uval (INITV_tc_val (types));
if (index <= 0) continue;
Index: osprey/ipa/local/ipl_summarize_template.h
===================================================================
--- osprey/ipa/local/ipl_summarize_template.h (revision 3512)
+++ osprey/ipa/local/ipl_summarize_template.h (working copy)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009-2010 Advanced Micro Devices, Inc. All Rights Reserved.
+ * Copyright (C) 2009-2011 Advanced Micro Devices, Inc. All Rights Reserved.
*/
/*
@@ -982,21 +982,34 @@
{
INITV_IDX st_entry = INITV_blk (blk);
ST_IDX st_idx = 0;
- if (INITV_kind (st_entry) != INITVKIND_ZERO)
+ bool catch_all = false;
+ if (INITV_kind (st_entry) == INITVKIND_ONE)
{
+ catch_all = true;
+ }
+ else if (INITV_kind (st_entry) != INITVKIND_ZERO)
+ {
st_idx = TCON_uval (INITV_tc_val (st_entry));
FmtAssert (st_idx != 0, ("Invalid st idx"));
}
- if (st_idx <= 0)
+ if (st_idx <= 0 && !catch_all)
{
blk = INITV_next (blk);
continue;
}
- INT32 index = Get_symbol_index (&St_Table [st_idx]);
INITV_IDX filter = INITV_next (st_entry); // for backup
- FmtAssert (index >= 0, ("Unexpected summary id for eh symbol"));
- INITV_Set_VAL (Initv_Table[st_entry], Enter_tcon (
- Host_To_Targ (MTYPE_U4, index)), 1);
+ if (!catch_all)
+ {
+ INT32 index = Get_symbol_index (&St_Table [st_idx]);
+ FmtAssert (index >= 0, ("Unexpected summary id for eh symbol"));
+ INITV_Set_VAL (Initv_Table[st_entry], Enter_tcon (
+ Host_To_Targ (MTYPE_U4, index)), 1);
+ }
+ else
+ {
+ // copy catch-all marker
+ INITV_Set_ONE (Initv_Table[st_entry], MTYPE_U4, 1);
+ }
Set_INITV_next (st_entry, filter);
blk = INITV_next (blk);
} while (blk);
@@ -1055,7 +1068,7 @@
for (; types; types = INITV_next (types))
{
int sym = 0;
- if (INITV_kind (types) != INITVKIND_ZERO)
+ if (INITV_kind (types) == INITVKIND_VAL)
sym = TCON_uval (INITV_tc_val (types));
if (sym > 0)
{
------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
