Hi Edward,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 98b1cc82c4affc16f5598d4fa14b1858671b2263
commit: 55c1528f9b97ff3b7efad73e8f79627fc2efb298 sfc: fix field-spanning memcpy in selftest
date: 4 months ago
config: arm-randconfig-003-20231122 (https://download.01.org/0day-ci/archive/20231122/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231122/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/sfc/selftest.c:48:16: warning: field ip within 'struct efx_loopback_payload::(anonymous at drivers/net/ethernet/sfc/selftest.c:46:2)' is less aligned than 'struct iphdr' and is usually due to 'struct efx_loopback_payload::(anonymous at drivers/net/ethernet/sfc/selftest.c:46:2)' being packed, which can lead to unaligned accesses [-Wunaligned-access]
48 | struct iphdr ip;
| ^
>> drivers/net/ethernet/sfc/selftest.c:48:16: warning: field ip within 'struct efx_loopback_payload::(unnamed at drivers/net/ethernet/sfc/selftest.c:46:2)' is less aligned than 'struct iphdr' and is usually due to 'struct efx_loopback_payload::(unnamed at drivers/net/ethernet/sfc/selftest.c:46:2)' being packed, which can lead to unaligned accesses [-Wunaligned-access]
2 warnings generated.
--
>> drivers/net/ethernet/sfc/falcon/selftest.c:45:16: warning: field ip within 'struct ef4_loopback_payload::(anonymous at drivers/net/ethernet/sfc/falcon/selftest.c:43:2)' is less aligned than 'struct iphdr' and is usually due to 'struct ef4_loopback_payload::(anonymous at drivers/net/ethernet/sfc/falcon/selftest.c:43:2)' being packed, which can lead to unaligned accesses [-Wunaligned-access]
45 | struct iphdr ip;
| ^
>> drivers/net/ethernet/sfc/falcon/selftest.c:45:16: warning: field ip within 'struct ef4_loopback_payload::(unnamed at drivers/net/ethernet/sfc/falcon/selftest.c:43:2)' is less aligned than 'struct iphdr' and is usually due to 'struct ef4_loopback_payload::(unnamed at drivers/net/ethernet/sfc/falcon/selftest.c:43:2)' being packed, which can lead to unaligned accesses [-Wunaligned-access]
2 warnings generated.
vim +48 drivers/net/ethernet/sfc/selftest.c
93e5dfa59b0e26 drivers/net/ethernet/sfc/selftest.c Ben Hutchings 2012-02-28 37
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 38 /*
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 39 * Loopback test packet structure
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 40 *
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 41 * The self-test should stress every RSS vector, and unfortunately
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 42 * Falcon only performs RSS on TCP/UDP packets.
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 43 */
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 44 struct efx_loopback_payload {
cf60ed46962992 drivers/net/ethernet/sfc/selftest.c Edward Cree 2023-06-23 45 char pad[2]; /* Ensures ip is 4-byte aligned */
55c1528f9b97ff drivers/net/ethernet/sfc/selftest.c Edward Cree 2023-07-28 46 struct_group_attr(packet, __packed,
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 47 struct ethhdr header;
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 @48 struct iphdr ip;
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 49 struct udphdr udp;
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 50 __be16 iteration;
1d20a16062e771 drivers/net/ethernet/sfc/selftest.c David S. Miller 2015-04-17 51 char msg[64];
55c1528f9b97ff drivers/net/ethernet/sfc/selftest.c Edward Cree 2023-07-28 52 );
cf60ed46962992 drivers/net/ethernet/sfc/selftest.c Edward Cree 2023-06-23 53 } __packed __aligned(4);
55c1528f9b97ff drivers/net/ethernet/sfc/selftest.c Edward Cree 2023-07-28 54 #define EFX_LOOPBACK_PAYLOAD_LEN \
55c1528f9b97ff drivers/net/ethernet/sfc/selftest.c Edward Cree 2023-07-28 55 sizeof_field(struct efx_loopback_payload, packet)
3273c2e8c66a21 drivers/net/sfc/selftest.c Ben Hutchings 2008-05-07 56
:::::: The code at line 48 was first introduced by commit
:::::: 3273c2e8c66a21ae1c53b0c730ee937c6efde7e2 [netdrvr] sfc: sfc: Add self-test support
:::::: TO: Ben Hutchings <[email protected]>
:::::: CC: Jeff Garzik <[email protected]>
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 21/11/2023 21:25, kernel test robot wrote:
> Hi Edward,
>
> FYI, the error/warning still remains.
As I've argued previously, this is a false positive / compiler bug,
and there is no way to resolve it without making the code strictly
worse.
This:
>>> drivers/net/ethernet/sfc/selftest.c:48:16: warning: field ip within 'struct efx_loopback_payload::(anonymous at drivers/net/ethernet/sfc/selftest.c:46:2)' is less aligned than 'struct iphdr' and is usually due to 'struct efx_loopback_payload::(anonymous at drivers/net/ethernet/sfc/selftest.c:46:2)' being packed, which can lead to unaligned accesses [-Wunaligned-access]
is complaining about alignment within an anonymous struct, which
only ever appears embedded within a larger struct in a way which
maintains the correct alignment.
#ifdef RANT
Indeed, the only way we even *could* create an unaligned access
out of this code would be via a declaration like
typeof(*(((struct efx_loopback_payload *)0)->packet)) bad;
because *the struct is anonymous*. And if that happened, the
bad declaration would be the place to warn, both because it's
incredibly ugly and because it's the place that's actually
wrong. The struct definition itself is entirely *fine*.
The compiler should be able to detect that, and if it's not smart
enough to do so then it shouldn't be trying to warn in the first
place. Quoth Linus[1]:
"And if the compiler isn't good enough to do it, then the compiler
shouldn't be warning about something that it hasn't got a clue about."
The anonymous struct has to be there so that we can placate the
memcpy hardening, and it has to contain a struct iphdr at a
4n+2 offset because that's what shape the on-the-wire packet
*is*. To avoid the warning we would need to lose __packed and
memcpy all of the members in and out of the buffer individually
to explicitly-calculated offsets, which is worse code.
#endif
Either fix the compiler to not warn, or fix your automation to
ignore this instance of the warning.
-ed
[1]: https://yarchive.net/comp/linux/gcc.html#13
On Wed, Nov 22, 2023 at 04:15:49PM +0000, Edward Cree wrote:
> On 21/11/2023 21:25, kernel test robot wrote:
> > Hi Edward,
> >
> > FYI, the error/warning still remains.
>
> As I've argued previously, this is a false positive / compiler bug,
> and there is no way to resolve it without making the code strictly
> worse.
>
> This:
> >>> drivers/net/ethernet/sfc/selftest.c:48:16: warning: field ip within 'struct efx_loopback_payload::(anonymous at drivers/net/ethernet/sfc/selftest.c:46:2)' is less aligned than 'struct iphdr' and is usually due to 'struct efx_loopback_payload::(anonymous at drivers/net/ethernet/sfc/selftest.c:46:2)' being packed, which can lead to unaligned accesses [-Wunaligned-access]
> is complaining about alignment within an anonymous struct, which
> only ever appears embedded within a larger struct in a way which
> maintains the correct alignment.
>
> #ifdef RANT
>
> Indeed, the only way we even *could* create an unaligned access
> out of this code would be via a declaration like
> typeof(*(((struct efx_loopback_payload *)0)->packet)) bad;
> because *the struct is anonymous*. And if that happened, the
> bad declaration would be the place to warn, both because it's
> incredibly ugly and because it's the place that's actually
> wrong. The struct definition itself is entirely *fine*.
> The compiler should be able to detect that, and if it's not smart
> enough to do so then it shouldn't be trying to warn in the first
> place. Quoth Linus[1]:
>
> "And if the compiler isn't good enough to do it, then the compiler
> shouldn't be warning about something that it hasn't got a clue about."
>
> The anonymous struct has to be there so that we can placate the
> memcpy hardening, and it has to contain a struct iphdr at a
> 4n+2 offset because that's what shape the on-the-wire packet
> *is*. To avoid the warning we would need to lose __packed and
> memcpy all of the members in and out of the buffer individually
> to explicitly-calculated offsets, which is worse code.
>
> #endif
>
> Either fix the compiler to not warn, or fix your automation to
> ignore this instance of the warning.
Hi Edward,
Thanks a lot for the detailed explanation. We've configured the robot
to ignore the "unaligned access" warning for this specific case.
Best Regards,
Yujie
> -ed
>
> [1]: https://yarchive.net/comp/linux/gcc.html#13
>