From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 0379E385841F; Thu, 11 Apr 2024 08:16:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0379E385841F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1712823399; bh=4zahxxXC08rucfyA0Cv5rc3L+E/UGVzmlQPgeGy/o08=; h=From:To:Subject:Date:In-Reply-To:References:From; b=iCjmFO6hF+Rv4UfKqzahKTXjOvLdwIKcFLTkIp+DioKufPgNFYtf8bXqxrUHSa24m qDkEjcnFXkPE3j9bsJ6Npfp+CwKLJ6+qSvA7AKzE7ZhNThZ0jJtgyRrnYTgTjgTYeU HTkxA37IQSu+gvCqkB3yBkZJOHyr9a22iwJ7F5Zs= From: "jakub at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug middle-end/110027] [11/12/13/14 regression] Stack objects with extended alignments (vectors etc) misaligned on detect_stack_use_after_return Date: Thu, 11 Apr 2024 08:16:35 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: middle-end X-Bugzilla-Version: 13.1.1 X-Bugzilla-Keywords: wrong-code X-Bugzilla-Severity: normal X-Bugzilla-Who: jakub at gcc dot gnu.org X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: jakub at gcc dot gnu.org X-Bugzilla-Target-Milestone: 11.5 X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D110027 --- Comment #20 from Jakub Jelinek --- (In reply to Hongtao Liu from comment #19) > (In reply to Jakub Jelinek from comment #17) > > Both of the posted patches are incorrect, this needs to be fixed in > > asan_emit_stack_protection, account for the different offsets[0] which > > happens when a stack pointer guard is created. > > I'll deal with it tomorrow. >=20 > It seems to me that the only offend place is where I've modifed, are there > other places where align_frame_offset (ASAN_RED_ZONE_SIZE) is also added? >=20 > Also, your patch adds a gcc_assert for offset[0], which seems to me there > was an assumption that offset[0] should be a multiple of alignb, thus mak= ing > my patch more reasonable? Your first patch aligned offsets[0] to maximum of alignb and ASAN_RED_ZONE_SIZE. But as I wrote in the reply to that mail, alignb there is the alignment of = just a single variable which is the first one to appear in the sorted list and is placed in the highest spot in the stack frame. That is not necessarily the largest alignment, the sorting ensures that it = is a variable with the largest size in the frame (and only if several of them ha= ve equal size, largest alignment from the same sized ones). Your second patch used maximum of BIGGEST_ALIGNMENT / BITS_PER_UNIT and ASAN_RED_ZONE_SIZE. That doesn't change anything at all when using -mno-avx512f - offsets[0] is still just 32-byte aligned in that case relati= ve to top of frame, just changes the -mavx512f case to be 64-byte aligned offsets[0] (aka offsets[0] is then either 0 or -64 instead of either 0 or -= 32). That will not help if any variable in the frame needs 128-byte, 256-byte, 512-byte ... 4096-byte alignment. If you want to fix the bug in the spot you've touched, you'd need to walk a= ll the stack_vars[stack_vars_sorted[si2]] for si2 [si + 1, n - 1] and for those wh= ere the loop would do anything (i.e. stack_vars[i2].representative =3D=3D i2 && TREE_CODE (decl2) =3D=3D SSA_NAME ? SA.partition_to_pseudo[var_to_partition (SA.map, decl2)] =3D=3D NULL_R= TX : DECL_RTL (decl2) =3D=3D pc_rtx and the pred applies (but that means also walking the earlier ones! because with -fstack-protector* the vars can be processed in several calls) and alignb2 * BITS_PER_UNIT <=3D MAX_SUPPORTED_STACK_ALIGNMENT and compute maximum of those alignments. That maximum is already computed, data->asan_alignb =3D MAX (data->asan_alignb, alignb); computes that, but you get the final result only after you do all the expand_stack_vars calls. You'd need to compute it before. Though, that change would be still in the wrong place. The thing is, it would be a waste of the precious stack space when it isn't needed at all (e.g. when asan will not at compile time do the use after ret= urn checking, or if it won't do it at runtime, or even if it will do at runtime= it will waste the space on the stack). My patch enlarges if needed just the __asan_stack_malloc_N allocated chunk,= not the stack frame size.=