From patchwork Thu Jul 15 16:05:45 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 58992 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 9A006B6F0C for ; Fri, 16 Jul 2010 02:04:20 +1000 (EST) Received: (qmail 18860 invoked by alias); 15 Jul 2010 16:04:15 -0000 Received: (qmail 18448 invoked by uid 22791); 15 Jul 2010 16:04:12 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 15 Jul 2010 16:04:05 +0000 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o6FG43Or023100 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 15 Jul 2010 12:04:04 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [10.16.42.4]) by int-mx08.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o6FG42AB010450 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 15 Jul 2010 12:04:03 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [127.0.0.1]) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4) with ESMTP id o6FG5mBg022891; Thu, 15 Jul 2010 18:05:48 +0200 Received: (from jakub@localhost) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4/Submit) id o6FG5jD2022707; Thu, 15 Jul 2010 18:05:45 +0200 Date: Thu, 15 Jul 2010 18:05:45 +0200 From: Jakub Jelinek To: Uros Bizjak , Richard Henderson , Jan Hubicka Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix va_start on x86_64 after any named argument passed on the stack that needed padding (PR target/44942) Message-ID: <20100715160545.GQ20208@tyan-ft48-01.lab.bos.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-12-10) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Hi! When say one integer argument is passed on the stack (because all regs are already used by earlier arguments) and then long double is passed, it is padded on the stack to 16 byte boundary on x86-64 (similarly for a bunch of other argument types). Unfortunately cum->words isn't adjusted for this padding and thus ap.overflow_arg_area is set to a wrong address. It seems cum->words is only ever used by ix86_va_start (for -m64 only and not for MS ABI), so this patch shouldn't affect anything but __builtin_va_start expansion. Bootstrapped/regtested on x86_64-linux and i686-linux, tested with compat.exp and struct-layout-1.exp also against older gcc in ALT_CC_UNDER_TEST/ALT_CXX_UNDER_TEST. Ok for trunk? What about release branches after a while on the trunk? While this probably isn't a regression, it is quite bad wrong code bug. 2010-07-15 Jakub Jelinek PR target/44942 * config/i386/i386-protos.h (ix86_function_arg_boundary): Change second argument to const_tree. * config/i386/i386.c (function_arg_advance): If padding needs to be inserted before argument, increment cum->words by number of padding words as well. (contains_aligned_value_p): Change argument to const_tree. (ix86_function_arg_boundary): Change second argument to const_tree. * gcc.c-torture/execute/pr44942.c: New test. * gcc.target/i386/pr44942.c: New test. Jakub --- gcc/config/i386/i386-protos.h.jj 2010-07-13 15:56:31.000000000 +0200 +++ gcc/config/i386/i386-protos.h 2010-07-15 12:45:01.000000000 +0200 @@ -140,8 +140,8 @@ extern enum machine_mode ix86_fp_compare extern rtx ix86_libcall_value (enum machine_mode); extern bool ix86_function_arg_regno_p (int); extern void ix86_asm_output_function_label (FILE *, const char *, tree); -extern int ix86_function_arg_boundary (enum machine_mode, tree); -extern bool ix86_solaris_return_in_memory (const_tree,const_tree); +extern int ix86_function_arg_boundary (enum machine_mode, const_tree); +extern bool ix86_solaris_return_in_memory (const_tree, const_tree); extern rtx ix86_force_to_memory (enum machine_mode, rtx); extern void ix86_free_from_memory (enum machine_mode); extern enum calling_abi ix86_cfun_abi (void); --- gcc/config/i386/i386.c.jj 2010-07-13 15:56:31.000000000 +0200 +++ gcc/config/i386/i386.c 2010-07-15 12:44:31.000000000 +0200 @@ -6157,9 +6157,8 @@ function_arg_advance_64 (CUMULATIVE_ARGS if (!named && VALID_AVX256_REG_MODE (mode)) return; - if (!examine_argument (mode, type, 0, &int_nregs, &sse_nregs)) - cum->words += words; - else if (sse_nregs <= cum->sse_nregs && int_nregs <= cum->nregs) + if (examine_argument (mode, type, 0, &int_nregs, &sse_nregs) + && sse_nregs <= cum->sse_nregs && int_nregs <= cum->nregs) { cum->nregs -= int_nregs; cum->sse_nregs -= sse_nregs; @@ -6167,7 +6166,11 @@ function_arg_advance_64 (CUMULATIVE_ARGS cum->sse_regno += sse_nregs; } else - cum->words += words; + { + int align = ix86_function_arg_boundary (mode, type) / BITS_PER_WORD; + cum->words = (cum->words + align - 1) & ~(align - 1); + cum->words += words; + } } static void @@ -6508,7 +6511,7 @@ ix86_pass_by_reference (CUMULATIVE_ARGS /* Return true when TYPE should be 128bit aligned for 32bit argument passing ABI. */ static bool -contains_aligned_value_p (tree type) +contains_aligned_value_p (const_tree type) { enum machine_mode mode = TYPE_MODE (type); if (((TARGET_SSE && SSE_REG_MODE_P (mode)) @@ -6558,7 +6561,7 @@ contains_aligned_value_p (tree type) specified mode and type. */ int -ix86_function_arg_boundary (enum machine_mode mode, tree type) +ix86_function_arg_boundary (enum machine_mode mode, const_tree type) { int align; if (type) --- gcc/testsuite/gcc.c-torture/execute/pr44942.c.jj 2010-07-15 13:41:28.000000000 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr44942.c 2010-07-15 13:46:40.000000000 +0200 @@ -0,0 +1,70 @@ +/* PR target/44942 */ + +#include + +void +test1 (int a, int b, int c, int d, int e, int f, int g, long double h, ...) +{ + int i; + va_list ap; + + va_start (ap, h); + i = va_arg (ap, int); + if (i != 1234) + __builtin_abort (); + va_end (ap); +} + +void +test2 (int a, int b, int c, int d, int e, int f, int g, long double h, int i, + long double j, int k, long double l, int m, long double n, ...) +{ + int o; + va_list ap; + + va_start (ap, n); + o = va_arg (ap, int); + if (o != 1234) + __builtin_abort (); + va_end (ap); +} + +void +test3 (double a, double b, double c, double d, double e, double f, + double g, long double h, ...) +{ + double i; + va_list ap; + + va_start (ap, h); + i = va_arg (ap, double); + if (i != 1234.0) + __builtin_abort (); + va_end (ap); +} + +void +test4 (double a, double b, double c, double d, double e, double f, double g, + long double h, double i, long double j, double k, long double l, + double m, long double n, ...) +{ + double o; + va_list ap; + + va_start (ap, n); + o = va_arg (ap, double); + if (o != 1234.0) + __builtin_abort (); + va_end (ap); +} + +int +main () +{ + test1 (0, 0, 0, 0, 0, 0, 0, 0.0L, 1234); + test2 (0, 0, 0, 0, 0, 0, 0, 0.0L, 0, 0.0L, 0, 0.0L, 0, 0.0L, 1234); + test3 (0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0L, 1234.0); + test4 (0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0L, 0.0, 0.0L, + 0.0, 0.0L, 0.0, 0.0L, 1234.0); + return 0; +} --- gcc/testsuite/gcc.target/i386/pr44942.c.jj 2010-07-15 13:52:37.000000000 +0200 +++ gcc/testsuite/gcc.target/i386/pr44942.c 2010-07-15 13:53:24.000000000 +0200 @@ -0,0 +1,44 @@ +/* PR target/44942 */ +/* { dg-do run { target lp64 } } */ + +#include +#include + +void +test1 (double a, double b, double c, double d, double e, double f, + double g, __m128d h, ...) +{ + double i; + va_list ap; + + va_start (ap, h); + i = va_arg (ap, double); + if (i != 1234.0) + __builtin_abort (); + va_end (ap); +} + +void +test2 (double a, double b, double c, double d, double e, double f, double g, + __m128d h, double i, __m128d j, double k, __m128d l, + double m, __m128d n, ...) +{ + double o; + va_list ap; + + va_start (ap, n); + o = va_arg (ap, double); + if (o != 1234.0) + __builtin_abort (); + va_end (ap); +} + +int +main () +{ + __m128d m = _mm_set1_pd (7.0); + test1 (0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, m, 1234.0); + test2 (0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, m, 0.0, m, + 0.0, m, 0.0, m, 1234.0); + return 0; +}