Messages in this thread |  | | Date | Sun, 17 Jul 2022 16:45:08 -0500 | From | Segher Boessenkool <> | Subject | Re: mainline build failure of powerpc allmodconfig for prom_init_check |
| |
On Sun, Jul 17, 2022 at 02:11:52PM -0700, Linus Torvalds wrote: > On Sun, Jul 17, 2022 at 2:00 PM Segher Boessenkool > <segher@kernel.crashing.org> wrote: > > Calling mem* on a volatile object (or a struct containing one) is not > > valid. I opened gcc.gnu.org/PR106335. > > Well, that very quickly got marked as a duplicate of a decade-old bug. > > So I guess we shouldn't expect this to be fixed any time soon.
It shouldn't be all that hard to implement. GCC wants all ports to define their own mem* because these functions are so critical for performance, but it isn't hard to do a straightforward by-field copy for assignments if using memcpy would not be valid at all. Also, if we would have this we could make a compiler flag saying to always open-code this, getting rid of this annoyance (namely, that extetnal mem* are required) for -ffreestanding.
> That said, your test-case of copying the whole structure is very > different from the one in the kernel that works on them one structure > member at a time. > > I can *kind of* see the logic that when you do a whole struct > assignment, it turns into a "memcpy" without regard for volatile > members. You're not actually accessing the volatile members in some > particular order, so the struct assignment arguably does not really > have an access ordering that needs to be preserved.
The order is not defined, correct. But a "volatile int" can only be accessed as an int, and an external memcpy will typically use different size accesses, and can even access some fields more than once (or partially); all not okay for a volatile object.
> But the kernel code in question very much does access the members > individually, and so I think that the compiler quite unequivocally did > something horribly horribly bad by turning them into a memset. > > So I don't think your test-case is really particularly good, and maybe > that's why that old bug has languished for over a decade - people > didn't realize just *how* incredibly broken it was.
People haven't looked at my test case for all that time, it sprouted from my demented mind just minutes ago ;-) The purpose of writing it this way was to make sure that memcpy will be called for this (on any target etc.), not some shorter and/or smarter thing.
I don't know what the real reason is that this bugs hasn't been fixed yet. It should be quite easy to make this more correct. In <https://patchwork.ozlabs.org/project/gcc/patch/1408617247-21558-1-git-send-email-james.greenhalgh@arm.com/#843066> Richard suggested doing it in the frontend, which seems reasonable (but more work than the patch there).
There have been no follow-up patches as far as I can see :-(
Segher
|  |