* Intel i40e code finds smatch bug
@ 2021-01-19 17:22 Dan McDonald
2021-01-19 18:55 ` Dan Carpenter
0 siblings, 1 reply; 10+ messages in thread
From: Dan McDonald @ 2021-01-19 17:22 UTC (permalink / raw)
To: smatch; +Cc: Dan McDonald
[-- Attachment #1: Type: text/plain, Size: 703 bytes --]
We in illumos are in the midst of updating our i40e code from Intel upstream. We found a bug in smatch while building it. It's documented here in:
https://www.illumos.org/issues/13457
A colleague recommended I include a toy program, try the smatch `master` branch on a Linux distro, and submit the toy to this list. Its source is attached, ready to compile (without incident) and smatch (with one incident, but not two). You'll see two cases of potential failure, but only one (the one Intel source uses to confirm proper compilation) breaks smatch. Look for the KEBE comments.
Once this bug gets fixed, I suspect we'll also be updating our branch of smatch as well. :)
Thanks,
Dan
[-- Attachment #2: pack.c --]
[-- Type: application/octet-stream, Size: 1646 bytes --]
#include <sys/types.h>
#include <inttypes.h>
#include <stdio.h>
#define u8 uint8_t
#define __le16 uint16_t
#define __le32 uint32_t
#pragma pack(1)
/* Run PHY Activity (0x0626) */
struct i40e_aqc_run_phy_activity {
u8 cmd_flags;
__le16 activity_id;
#define I40E_AQ_RUN_PHY_ACT_ID_USR_DFND 0x10
u8 reserved;
union {
struct {
__le32 dnl_opcode;
#define I40E_AQ_RUN_PHY_ACT_DNL_OPCODE_GET_EEE_STAT_DUR 0x801a
#define I40E_AQ_RUN_PHY_ACT_DNL_OPCODE_GET_EEE_STAT 0x801b
#define I40E_AQ_RUN_PHY_ACT_DNL_OPCODE_GET_EEE_DUR 0x1801b
__le32 data;
u8 reserved2[4];
} cmd;
struct {
__le32 cmd_status;
#define I40E_AQ_RUN_PHY_ACT_CMD_STAT_SUCC 0x4
#define I40E_AQ_RUN_PHY_ACT_CMD_STAT_MASK 0xFFFF
__le32 data0;
__le32 data1;
} resp;
} params;
};
#pragma pack()
#define I40E_CHECK_STRUCT_LEN(n, X) enum i40e_static_assert_enum_##X \
{ i40e_static_assert_##X = (n)/((sizeof(struct X) == (n)) ? 1 : 0) }
/* XXX KEBE SAYS this one fails! */
I40E_CHECK_STRUCT_LEN(16, i40e_aqc_run_phy_activity);
int
main(void)
{
struct i40e_aqc_run_phy_activity foo;
printf("foo == 0x%p, foo.activity_id = 0x%p, size == %d, tripval = %d\n",
&foo, &foo.activity_id, sizeof (struct i40e_aqc_run_phy_activity),
/* XXX KEBE SAYS but this one doesn't. */
16 / ((sizeof (struct i40e_aqc_run_phy_activity) == 16) ? 1 : 0) );
return (0);
}
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: Intel i40e code finds smatch bug
2021-01-19 17:22 Intel i40e code finds smatch bug Dan McDonald
@ 2021-01-19 18:55 ` Dan Carpenter
2021-01-19 19:12 ` Dan McDonald
2021-01-19 20:51 ` Dan McDonald
0 siblings, 2 replies; 10+ messages in thread
From: Dan Carpenter @ 2021-01-19 18:55 UTC (permalink / raw)
To: Dan McDonald; +Cc: smatch
On Tue, Jan 19, 2021 at 12:22:50PM -0500, Dan McDonald wrote:
> We in illumos are in the midst of updating our i40e code from Intel upstream. We found a bug in smatch while building it. It's documented here in:
>
> https://www.illumos.org/issues/13457
>
> A colleague recommended I include a toy program, try the smatch `master` branch on a Linux distro, and submit the toy to this list. Its source is attached, ready to compile (without incident) and smatch (with one incident, but not two). You'll see two cases of potential failure, but only one (the one Intel source uses to confirm proper compilation) breaks smatch. Look for the KEBE comments.
>
> Once this bug gets fixed, I suspect we'll also be updating our branch of smatch as well. :)
>
> Thanks,
> Dan
>
Unfortunately Sparse doesn't support #pragma pack. This was reported to
the Sparse list last February and the response was that pragma pack is
disgusting so don't use it.
http://tw.spinics.net/lists/linux-sparse/msg10816.html
I wish I could give you better news than that...
Luk recently was working on better support of packed structs (which have
been a known issue for a while) but I doubt that will help here.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Intel i40e code finds smatch bug
2021-01-19 18:55 ` Dan Carpenter
@ 2021-01-19 19:12 ` Dan McDonald
2021-01-19 20:51 ` Dan McDonald
1 sibling, 0 replies; 10+ messages in thread
From: Dan McDonald @ 2021-01-19 19:12 UTC (permalink / raw)
To: Dan Carpenter; +Cc: smatch, Dan McDonald
On Jan 19, 2021, at 1:55 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Tue, Jan 19, 2021 at 12:22:50PM -0500, Dan McDonald wrote:
>> We in illumos are in the midst of updating our i40e code from Intel upstream. We found a bug in smatch while building it. It's documented here in:
>>
>> https://www.illumos.org/issues/13457
>>
>> A colleague recommended I include a toy program, try the smatch `master` branch on a Linux distro, and submit the toy to this list. Its source is attached, ready to compile (without incident) and smatch (with one incident, but not two). You'll see two cases of potential failure, but only one (the one Intel source uses to confirm proper compilation) breaks smatch. Look for the KEBE comments.
>>
>> Once this bug gets fixed, I suspect we'll also be updating our branch of smatch as well. :)
>>
>> Thanks,
>> Dan
>>
>
> Unfortunately Sparse doesn't support #pragma pack. This was reported to
> the Sparse list last February and the response was that pragma pack is
> disgusting so don't use it.
>
> http://tw.spinics.net/lists/linux-sparse/msg10816.html
Heh heh.
> I wish I could give you better news than that...
>
> Luk recently was working on better support of packed structs (which have
> been a known issue for a while) but I doubt that will help here.
I saw his followup on the archive pointer you gave me (thanks for that, too, as I couldn't find it with some cursory Googling).
We have a workaround in place, since it's only in one (OEM-supplied) driver.
Thanks!
Dan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Intel i40e code finds smatch bug
2021-01-19 18:55 ` Dan Carpenter
2021-01-19 19:12 ` Dan McDonald
@ 2021-01-19 20:51 ` Dan McDonald
2021-01-19 21:42 ` Luc Van Oostenryck
1 sibling, 1 reply; 10+ messages in thread
From: Dan McDonald @ 2021-01-19 20:51 UTC (permalink / raw)
To: Dan Carpenter; +Cc: smatch, Dan McDonald
On Jan 19, 2021, at 1:55 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Luk recently was working on better support of packed structs (which have
> been a known issue for a while) but I doubt that will help here.
I tried using packed struct attributes instead of the pragma and no luck there either:
centos(~)[0]% diff -u pack.c pack2.c
--- pack.c 2021-01-19 17:16:13.258512270 +0000
+++ pack2.c 2021-01-19 20:49:45.886696658 +0000
@@ -6,9 +6,8 @@
#define __le16 uint16_t
#define __le32 uint32_t
-#pragma pack(1)
/* Run PHY Activity (0x0626) */
-struct i40e_aqc_run_phy_activity {
+struct __attribute__((packed)) i40e_aqc_run_phy_activity {
u8 cmd_flags;
__le16 activity_id;
#define I40E_AQ_RUN_PHY_ACT_ID_USR_DFND 0x10
@@ -31,7 +30,6 @@
} resp;
} params;
};
-#pragma pack()
#define I40E_CHECK_STRUCT_LEN(n, X) enum i40e_static_assert_enum_##X \
{ i40e_static_assert_##X = (n)/((sizeof(struct X) == (n)) ? 1 : 0) }
centos(~)[1]%
Dan
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: Intel i40e code finds smatch bug
2021-01-19 20:51 ` Dan McDonald
@ 2021-01-19 21:42 ` Luc Van Oostenryck
2021-01-19 21:49 ` Dan McDonald
2021-01-21 8:49 ` Dan Carpenter
0 siblings, 2 replies; 10+ messages in thread
From: Luc Van Oostenryck @ 2021-01-19 21:42 UTC (permalink / raw)
To: Dan McDonald; +Cc: Dan Carpenter, smatch
On Tue, Jan 19, 2021 at 03:51:57PM -0500, Dan McDonald wrote:
> On Jan 19, 2021, at 1:55 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > Luk recently was working on better support of packed structs (which have
> > been a known issue for a while) but I doubt that will help here.
Indeed, the support for packed structs (via the attribute) is now
complete in Sparse and was mainlined 2 weeks ago or so and ...
> I tried using packed struct attributes instead of the pragma and no luck there either:
now it just needs to be merged into smatch.
-- Luc
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Intel i40e code finds smatch bug
2021-01-19 21:42 ` Luc Van Oostenryck
@ 2021-01-19 21:49 ` Dan McDonald
2021-01-21 8:49 ` Dan Carpenter
1 sibling, 0 replies; 10+ messages in thread
From: Dan McDonald @ 2021-01-19 21:49 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Dan McDonald, smatch
On Jan 19, 2021, at 4:42 PM, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote:
>
> On Tue, Jan 19, 2021 at 03:51:57PM -0500, Dan McDonald wrote:
>> On Jan 19, 2021, at 1:55 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>>>
>>> Luk recently was working on better support of packed structs (which have
>>> been a known issue for a while) but I doubt that will help here.
>
> Indeed, the support for packed structs (via the attribute) is now
> complete in Sparse and was mainlined 2 weeks ago or so and ...
>
>> I tried using packed struct attributes instead of the pragma and no luck there either:
>
> now it just needs to be merged into smatch.
Well that's good news! Of course, one may have to mention this to Intel in regards to i40e, but that's not this mailing list's problem. Nor is having illumos update its downstream with this as well. :)
Thank you!
Dan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Intel i40e code finds smatch bug
2021-01-19 21:42 ` Luc Van Oostenryck
2021-01-19 21:49 ` Dan McDonald
@ 2021-01-21 8:49 ` Dan Carpenter
2021-01-25 15:10 ` Dan Carpenter
1 sibling, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2021-01-21 8:49 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Dan McDonald, smatch
On Tue, Jan 19, 2021 at 10:42:21PM +0100, Luc Van Oostenryck wrote:
> On Tue, Jan 19, 2021 at 03:51:57PM -0500, Dan McDonald wrote:
> > On Jan 19, 2021, at 1:55 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > >
> > > Luk recently was working on better support of packed structs (which have
> > > been a known issue for a while) but I doubt that will help here.
>
> Indeed, the support for packed structs (via the attribute) is now
> complete in Sparse and was mainlined 2 weeks ago or so and ...
>
> > I tried using packed struct attributes instead of the pragma and no luck there either:
>
> now it just needs to be merged into smatch.
>
Awesome! I've merged it and will push the merge tomorrow if the tests
run well tonight. Thanks!
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Intel i40e code finds smatch bug
2021-01-21 8:49 ` Dan Carpenter
@ 2021-01-25 15:10 ` Dan Carpenter
2021-01-25 16:33 ` Luc Van Oostenryck
2021-01-25 16:41 ` Dan McDonald
0 siblings, 2 replies; 10+ messages in thread
From: Dan Carpenter @ 2021-01-25 15:10 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Dan McDonald, smatch
Pushed!
Thanks, everyone.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Intel i40e code finds smatch bug
2021-01-25 15:10 ` Dan Carpenter
@ 2021-01-25 16:33 ` Luc Van Oostenryck
2021-01-25 16:41 ` Dan McDonald
1 sibling, 0 replies; 10+ messages in thread
From: Luc Van Oostenryck @ 2021-01-25 16:33 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Dan McDonald, smatch
On Mon, Jan 25, 2021 at 06:10:19PM +0300, Dan Carpenter wrote:
> Pushed!
Great. Thank you.
- Luc
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Intel i40e code finds smatch bug
2021-01-25 15:10 ` Dan Carpenter
2021-01-25 16:33 ` Luc Van Oostenryck
@ 2021-01-25 16:41 ` Dan McDonald
1 sibling, 0 replies; 10+ messages in thread
From: Dan McDonald @ 2021-01-25 16:41 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Luc Van Oostenryck, smatch
> On Jan 25, 2021, at 10:10 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Pushed!
>
> Thanks, everyone.
Thank you!
Dan
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-01-25 18:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-19 17:22 Intel i40e code finds smatch bug Dan McDonald
2021-01-19 18:55 ` Dan Carpenter
2021-01-19 19:12 ` Dan McDonald
2021-01-19 20:51 ` Dan McDonald
2021-01-19 21:42 ` Luc Van Oostenryck
2021-01-19 21:49 ` Dan McDonald
2021-01-21 8:49 ` Dan Carpenter
2021-01-25 15:10 ` Dan Carpenter
2021-01-25 16:33 ` Luc Van Oostenryck
2021-01-25 16:41 ` Dan McDonald
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.