[Bug target/61656] Undefined behavior in classify_argument

["ubizjak at gmail dot com" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61656

--- Comment #1 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Jakub Jelinek from comment #0)
> /usr/src/gcc/obj052/gcc/xgcc -B/usr/src/gcc/obj052/gcc/
> -fno-diagnostics-show-caret -fdiagnostics-color=never -O0 -w -c -o pr4
> 2025-2.o /usr/src/gcc/gcc/testsuite/gcc.c-torture/compile/pr42025-2.c
> ../../gcc/config/i386/i386.c:6583:60: runtime error: mload of value 32669,
> which is not a valid value for type 'x86_64_reg_class'
> 
> This is on passing
> typedef struct { void *p; } Ptr;
> struct A { int i; union { Ptr p; char *q; } u; };
> by value and the problem is that when processing the union with bit_offset
> 64,
> words is 1 (u is DImode 64-bit field), but when we recurse, we are called
> with
> 64-bit scalar DImode q and bit_offset 64, that is size (128-1)&0x7f and so
> it is the size < 64+64 case where we return 2 and { X86_64_INTEGER_CLASS,
> X86_64_INTEGER_CLASS }; in subclauses.  But words is 1 and we merge classes
> up to num (2).  A simple fix could be:
>    if (!num)
>      return 0;
> -  for (i = 0; i < num; i++)
> +  for (i = 0; i < num && i < words; i++)
>      classes[i] = merge_classes (subclasses[i], classes[i]);
> in the UNION_TYPE case, as it seems the caller will not care about classes
> above returned number (words).  I'd hope such a patch should not change the
> ABI even.
> I don't know whether there isn't an ABI problem related to this though, say
> if at bit_offset 64 we have just SImode field in the union rather than
> DImode,
> then I'd guess the recursive call would give us { X86_64_INTEGER_CLASS,
> X86_64_INTEGERSI_CLASS }; but we'd use X86_64_INTEGER_CLASS anyway, as we
> are looking at position 0, not 1.

X86_64_INTEGER_CLASS and X86_64_INTEGERSI_CLASS are handled in the same way,
the only difference is that the later class allows/uses cheaper movl insn. So,
if a SImode value lives at bit_offset 64, the change from INTEGER_CLASS to
INTEGERSI_CLASS does NOT introduce an ABI change - movl just narrows memory
load and sets "don't care" high bits to zero.
>From gcc-bugs-return-455678-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Jul 04 12:21:58 2014
Return-Path: <gcc-bugs-return-455678-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 13185 invoked by alias); 4 Jul 2014 12:21:57 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 13092 invoked by uid 48); 4 Jul 2014 12:21:47 -0000
From: "matz at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/61203] [4.8/4.9/4.10 Regression] g++.old-deja/g++.jason/rvalue2.C FAILs with -O2 -fno-inline
Date: Fri, 04 Jul 2014 12:21:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: tree-optimization
X-Bugzilla-Version: 4.10.0
X-Bugzilla-Keywords: wrong-code
X-Bugzilla-Severity: normal
X-Bugzilla-Who: matz at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 4.8.4
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-61203-4-3K0tVqMA4y@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-61203-4@http.gcc.gnu.org/bugzilla/>
References: <bug-61203-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-07/txt/msg00269.txt.bz2
Content-length: 917

https://gcc.gnu.org/bugzilla/show_bug.cgi?ida203

--- Comment #3 from Michael Matz <matz at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> same spot.  No idea what to do against this though, treating a CLOBBER as a
> barrier for propagation of addresses to other local variables would penalize
> stuff way too much.

Conceptually this should be the right thing.  The clobber says
the object doesn't exist anymore, so also forming its address should be
invalid/implementation defined.  Either the clobber should move or it should
be a barrier also for addresses of the clobbered object.  I'm not sure if that
really would penalize much.

> And giving up on stack slot sharing because of such an
> artificial testcase is not useful either.

Perhaps expansion could detect the situation (address of clobbered object
leaked after the clobber) and disable sharing just for such problematic
objects.