Bug 114404 - [11] GCC reorders stores when it probably shouldn't
Summary: [11] GCC reorders stores when it probably shouldn't
Status: RESOLVED DUPLICATE of bug 49330
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 11.4.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2024-03-20 10:53 UTC by Ilya Leoshkevich
Modified: 2024-03-20 13:48 UTC (History)
3 users (show)

See Also:
Host: s390x-linux-gnu
Target: s390x-linux-gnu
Build: s390x-linux-gnu
Known to work:
Known to fail:
Last reconfirmed:


Attachments
preprocessed source (256.40 KB, application/x-xz)
2024-03-20 10:54 UTC, Ilya Leoshkevich
Details
cc1 invocation (823 bytes, application/x-shellscript)
2024-03-20 10:54 UTC, Ilya Leoshkevich
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Leoshkevich 2024-03-20 10:53:14 UTC
Reproducible with gcc commit 1b5510a59163.
I'm writing this up as a result of the following linux kernel discussion:

https://lore.kernel.org/bpf/c9923c1d-971d-4022-8dc8-1364e929d34c@gmail.com/
https://lore.kernel.org/bpf/20240320015515.11883-1-iii@linux.ibm.com/

In the following code:

extern const char bpf_plt[];
extern const char bpf_plt_ret[];
extern const char bpf_plt_target[];
static void bpf_jit_plt(void *plt, void *ret, void *target)
{
	memcpy(plt, bpf_plt, BPF_PLT_SIZE);
	*(void **)((char *)plt + (bpf_plt_ret - bpf_plt)) = ret;
	*(void **)((char *)plt + (bpf_plt_target - bpf_plt)) = target ?: ret;
}

GCC 11's sched1 pass reorders memcpy() and assignments.  In GCC 12 this behavior is gone after

commit 2e96b5f14e4025691b57d2301d71aa6092ed44bc                                                          
Author: Aldy Hernandez <aldyh@redhat.com>                                                                
Date:   Tue Jun 15 12:32:51 2021 +0200                                                                   
                                                                                                         
    Backwards jump threader rewrite with ranger.

but this seems to be accidental.  Internally, output_dependence() for the respective mems returns false, because it believes that they are based on different SYMBOL_REFs.  This may be because on the C level we are not allowed to subtract pointers to different objects.

However, a possible solution to this should be casting pointers to longs, since C pointer subtraction rules would no longer apply, but in practice this does nothing. 

In the attached minimized preprocessed source with long casts we get:

        stg     %r3,232(%r2,%r15)
        ltgr    %r11,%r11
        locgrne %r3,%r11
        stg     %r3,232(%r1,%r15)
        la      %r2,0(%r1,%r9)
        la      %r3,232(%r1,%r15)
        mvc     232(16,%r15),0(%r5)
        mvc     248(16,%r15),16(%r5)
        lghi    %r4,8
        brasl   %r14,s390_kernel_write@PLT

so the assignments are placed before the memcpy().
Comment 1 Ilya Leoshkevich 2024-03-20 10:54:09 UTC
Created attachment 57744 [details]
preprocessed source
Comment 2 Ilya Leoshkevich 2024-03-20 10:54:34 UTC
Created attachment 57745 [details]
cc1 invocation
Comment 3 Richard Biener 2024-03-20 11:44:46 UTC
Sounds similar to PR113255.  Yes, the code is undefined and you should cast
the pointers to uintptr_t but for the RTL problem that won't help.  Note
the increment of 'plt' is similarly invalid so that has to be uintptr_t as
well.

It might be that different (no) REG_POINTER marking will then avoid the
latent PR113255 issue from triggering.

It might be that the s390 cpymem expander does some bogus REG_POINTER
marking as well (or that it lacks marking, causing the heuristics to
go wrong).

You can check whether backporting the PR113255 fix avoids the issue,
but I do not consider backporting the revs suitable.
Comment 4 Ilya Leoshkevich 2024-03-20 13:28:32 UTC
Thanks, cherry-picking https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=a98d5130a6dcff2ed4db371e500550134777b8cf helped both with the minimized testcase and the actual kernel bug.  What you describe there - reassociation causing a wrong base term to be selected - matches what I've seen during debugging as well.  So let's close this as a duplicate.
Comment 5 Richard Biener 2024-03-20 13:48:50 UTC
Duplicate then.

*** This bug has been marked as a duplicate of bug 49330 ***