| Message ID | 1458599726-11471-1-git-send-email-patrick@parcs.ath.cx |
|---|---|
| State | New |
| Headers | show |
OK. Jason
On Mon, Mar 21, 2016 at 06:35:26PM -0400, Patrick Palka wrote: https://gcc.gnu.org/ml/gcc-patches/2016-03/msg01221.html > gcc/cp/ChangeLog: > > PR c++/70096 > * pt.c (tsubst_decl): Clear the DECL_MODE of the new decl. > > gcc/testsuite/ChangeLog: > > PR c++/70096 > * g++.dg/template/ptrmem30.C: New test. Can we have this one on the gcc-5 and gcc-4.9 branches too, please? It fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70107, which is a similar case to pr70096 but has a testcase that fails on the branches. Backporting is simply a matter of applying the patch, but note followup patch to the testcase. I've bootstrapped and regression tested on powerpc64le-linux, both branches.
On Tue, Apr 12, 2016 at 6:38 AM, Alan Modra <amodra@gmail.com> wrote: > On Mon, Mar 21, 2016 at 06:35:26PM -0400, Patrick Palka wrote: > https://gcc.gnu.org/ml/gcc-patches/2016-03/msg01221.html >> gcc/cp/ChangeLog: >> >> PR c++/70096 >> * pt.c (tsubst_decl): Clear the DECL_MODE of the new decl. >> >> gcc/testsuite/ChangeLog: >> >> PR c++/70096 >> * g++.dg/template/ptrmem30.C: New test. > > Can we have this one on the gcc-5 and gcc-4.9 branches too, please? > It fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70107, which is > a similar case to pr70096 but has a testcase that fails on the > branches. > > Backporting is simply a matter of applying the patch, but note > followup patch to the testcase. I've bootstrapped and regression > tested on powerpc64le-linux, both branches. > > -- > Alan Modra > Australia Development Lab, IBM I would appreciate it if someone could backport it for me since I won't have time today. BTW I think PARM_DECLs also may have the same issue where after substitution the DECL_MODE of the specialized PARM_DECL may not correspond to the TYPE_MODE of its type.
On Tue, Apr 12, 2016 at 06:53:55AM -0400, Patrick Palka wrote: > I would appreciate it if someone could backport it for me since I > won't have time today. I'm happy to handle backporting and commit for you after approval.
On 04/12/2016 07:20 AM, Alan Modra wrote: > On Tue, Apr 12, 2016 at 06:53:55AM -0400, Patrick Palka wrote: >> I would appreciate it if someone could backport it for me since I >> won't have time today. > > I'm happy to handle backporting and commit for you after approval. OK. Jason
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index f9c5b0b..ebfc45b 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -12374,6 +12374,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) /* The initializer must not be expanded until it is required; see [temp.inst]. */ DECL_INITIAL (r) = NULL_TREE; + if (VAR_P (r)) + DECL_MODE (r) = VOIDmode; if (CODE_CONTAINS_STRUCT (TREE_CODE (t), TS_DECL_WRTL)) SET_DECL_RTL (r, NULL); DECL_SIZE (r) = DECL_SIZE_UNIT (r) = 0; diff --git a/gcc/testsuite/g++.dg/template/ptrmem30.C b/gcc/testsuite/g++.dg/template/ptrmem30.C new file mode 100644 index 0000000..923238b --- /dev/null +++ b/gcc/testsuite/g++.dg/template/ptrmem30.C @@ -0,0 +1,45 @@ +// PR c++/70096 +// { dg-do run } + +int read; + +struct Holder +{ + void foo () { read = data; } + int data; +}; + +void +poison_stack () +{ + volatile char a[256]; + __builtin_memset ((void *)a, 0xa, sizeof a); +} + +template <typename F> +void test1 () +{ + Holder h; + h.data = 42; + F Holder::*fptr = &Holder::foo; + (h.*fptr)(); +} + +template <typename F> +void test2 () +{ + Holder h; + h.data = 42; + F Holder::*fptr1 = &Holder::foo; + F Holder::*fptr2 = fptr1; + (h.*fptr2)(); +} + + +int main () +{ + poison_stack (); + test1<void()>(); + poison_stack (); + test2<void()>(); +}
Given template <typename T> void foo () { T A::*fptr; } We don't know whether fptr is a pointer-to-member-function or a pointer-to-data-member until we know what T is. But until then, fptr gets the type OFFSET_TYPE. If we later instantiate foo with e.g. T = void() then we know that fptr is a pointer-to-member-function and so we give the specialized copy of fptr a type which satisfies TYPE_PTRMEMFUNC_P (some RECORD_TYPE). The placeholder OFFSET_TYPE and the resulting RECORD_TYPE however have different sizes and modes (DI vs TI on x86_64), but the DECL_MODE of the specialized copy of fptr never gets updated accordingly. This stale DECL_MODE eventually leads the backend to miscompile copies to and from fptr. [ Some more analysis at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70096#c4 ] This patch makes tsubst_decl clear the DECL_MODE of the new decl so that the subsequent call to layout_type can update it accordingly. Tested on x86_64-pc-linux-gnu. Does this patch look OK to commit? gcc/cp/ChangeLog: PR c++/70096 * pt.c (tsubst_decl): Clear the DECL_MODE of the new decl. gcc/testsuite/ChangeLog: PR c++/70096 * g++.dg/template/ptrmem30.C: New test. --- gcc/cp/pt.c | 2 ++ gcc/testsuite/g++.dg/template/ptrmem30.C | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 gcc/testsuite/g++.dg/template/ptrmem30.C