| Message ID | 20201226175129.9621-1-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | support __packed struct | expand |
On 26/12/2020 17:51, Luc Van Oostenryck wrote: > During parsing, Sparse recognizes the attribute 'packed' but this > attribute is otherwise ignored for several reasons: > 1) the attribute in 'struct __attr { ... }' is wrongly handled as > belonging to the whole declaration but it should belong to the type, > 2) the attribute in 'struct <name> { ... } __attr;' is simply ignored, > 3) the layout of packed bitfields need special care. > > This series contains 2 parts: > 1) handling of type attributes > 2) correct layout of packed structs, including packed bitfields. > > > This series is also available for review and testing at: > git://git.kernel.org/pub/scm/devel/sparse/sparse-dev.git packed-v2 I left a couple of minor comments, but (apart from patch #16) this otherwise LGTM. Patch #16 also looks good, but I would need to study it a bit more than I have time available to be totally happy. It does not seem to be handling the 'lowering' of 'odd bit-sized' symbols created in the previous patch (to answer my own question), so I would have to apply the patches (or fetch the above branch) to study some more. Hope you had a good holiday. ATB, Ramsay Jones
On Mon, Dec 28, 2020 at 05:18:50PM +0000, Ramsay Jones wrote: > > > On 26/12/2020 17:51, Luc Van Oostenryck wrote: > > During parsing, Sparse recognizes the attribute 'packed' but this > > attribute is otherwise ignored for several reasons: > > 1) the attribute in 'struct __attr { ... }' is wrongly handled as > > belonging to the whole declaration but it should belong to the type, > > 2) the attribute in 'struct <name> { ... } __attr;' is simply ignored, > > 3) the layout of packed bitfields need special care. > > > > This series contains 2 parts: > > 1) handling of type attributes > > 2) correct layout of packed structs, including packed bitfields. > > > > > > This series is also available for review and testing at: > > git://git.kernel.org/pub/scm/devel/sparse/sparse-dev.git packed-v2 > > I left a couple of minor comments, but (apart from patch #16) this > otherwise LGTM. > > Patch #16 also looks good, but I would need to study it a bit more > than I have time available to be totally happy. It does not seem > to be handling the 'lowering' of 'odd bit-sized' symbols created in > the previous patch (to answer my own question), so I would have to > apply the patches (or fetch the above branch) to study some more. Well, patch #16 doesn't contain the lowering, it kinda just enables the last bits to support (without out-of-bound access) packed structures including packed bitfields. > Hope you had a good holiday. Thank you. In truth, it was strange and quiet, very quiet. I wish you all the best for 2021! -- Luc
On 12/26/2020 9:51 AM, Luc Van Oostenryck wrote: > During parsing, Sparse recognizes the attribute 'packed' but this > attribute is otherwise ignored for several reasons: > 1) the attribute in 'struct __attr { ... }' is wrongly handled as > belonging to the whole declaration but it should belong to the type, > 2) the attribute in 'struct <name> { ... } __attr;' is simply ignored, > 3) the layout of packed bitfields need special care. > > This series contains 2 parts: > 1) handling of type attributes > 2) correct layout of packed structs, including packed bitfields. > > > This series is also available for review and testing at: > git://git.kernel.org/pub/scm/devel/sparse/sparse-dev.git packed-v2 > This series looks good to me, and it resolves the issues I saw a few weeks ago with size. Thanks, Jake > > Changes since v1: > * fix layout of packed bitfields > > Luc Van Oostenryck (16): > add testcases for dubious enum values > add testcases for exotic enum values > add testcases for enum attributes > add testcases for type attributes > add testcases for packed structures > add testcases for packed bitfields > apply_ctype: use self-explanatory argument name > apply_ctype: reverse the order of arguments > apply_ctype: move up its declaration > struct-attr: prepare to handle attributes at the end of struct definitions (1) > struct-attr: prepare to handle attributes at the end of struct definitions (2) > struct-attr: prepare to handle attributes at the end of struct definitions (3) > struct-attr: fix type attribute like 'struct __attr { ... }' > struct-attr: fix: do not ignore struct/union/enum type attributes > packed: no out-of-bound access of packed bitfields > packed: add support for __packed struct > > linearize.c | 13 +++++- > parse.c | 71 ++++++++++++++++--------------- > symbol.c | 12 ++++-- > symbol.h | 2 + > validation/enum-type-dubious.c | 18 ++++++++ > validation/enum-type-exotic.c | 28 ++++++++++++ > validation/packed-bitfield0.c | 66 ++++++++++++++++++++++++++++ > validation/packed-bitfield1.c | 27 ++++++++++++ > validation/packed-bitfield2.c | 15 +++++++ > validation/packed-bitfield3.c | 28 ++++++++++++ > validation/packed-bitfield4.c | 18 ++++++++ > validation/packed-bitfield5.c | 20 +++++++++ > validation/packed-deref0.c | 23 ++++++++++ > validation/packed-struct.c | 32 ++++++++++++++ > validation/parsing/enum-attr.c | 29 +++++++++++++ > validation/type-attribute-align.c | 19 +++++++++ > validation/type-attribute-as.c | 33 ++++++++++++++ > validation/type-attribute-mod.c | 21 +++++++++ > validation/type-attribute-qual.c | 12 ++++++ > 19 files changed, 448 insertions(+), 39 deletions(-) > create mode 100644 validation/enum-type-dubious.c > create mode 100644 validation/enum-type-exotic.c > create mode 100644 validation/packed-bitfield0.c > create mode 100644 validation/packed-bitfield1.c > create mode 100644 validation/packed-bitfield2.c > create mode 100644 validation/packed-bitfield3.c > create mode 100644 validation/packed-bitfield4.c > create mode 100644 validation/packed-bitfield5.c > create mode 100644 validation/packed-deref0.c > create mode 100644 validation/packed-struct.c > create mode 100644 validation/parsing/enum-attr.c > create mode 100644 validation/type-attribute-align.c > create mode 100644 validation/type-attribute-as.c > create mode 100644 validation/type-attribute-mod.c > create mode 100644 validation/type-attribute-qual.c > > > base-commit: 1b896707d95982c7c9cdd5cd0ab4afd80f766a94 > Cc: Jacob Keller <jacob.e.keller@intel.com> >
On 12/28/2020 1:33 PM, Luc Van Oostenryck wrote: > On Mon, Dec 28, 2020 at 05:18:50PM +0000, Ramsay Jones wrote: >> >> >> On 26/12/2020 17:51, Luc Van Oostenryck wrote: >>> During parsing, Sparse recognizes the attribute 'packed' but this >>> attribute is otherwise ignored for several reasons: >>> 1) the attribute in 'struct __attr { ... }' is wrongly handled as >>> belonging to the whole declaration but it should belong to the type, >>> 2) the attribute in 'struct <name> { ... } __attr;' is simply ignored, >>> 3) the layout of packed bitfields need special care. >>> >>> This series contains 2 parts: >>> 1) handling of type attributes >>> 2) correct layout of packed structs, including packed bitfields. >>> >>> >>> This series is also available for review and testing at: >>> git://git.kernel.org/pub/scm/devel/sparse/sparse-dev.git packed-v2 >> >> I left a couple of minor comments, but (apart from patch #16) this >> otherwise LGTM. >> >> Patch #16 also looks good, but I would need to study it a bit more >> than I have time available to be totally happy. It does not seem >> to be handling the 'lowering' of 'odd bit-sized' symbols created in >> the previous patch (to answer my own question), so I would have to >> apply the patches (or fetch the above branch) to study some more. > > Well, patch #16 doesn't contain the lowering, it kinda just enables > the last bits to support (without out-of-bound access) packed structures > including packed bitfields. > What does the 'lowering' gain us? Or, in other words, what is still missing after this series? >> Hope you had a good holiday. > > Thank you. In truth, it was strange and quiet, very quiet. > > I wish you all the best for 2021! > > -- Luc >
On Tue, Jan 05, 2021 at 09:56:05AM -0800, Jacob Keller wrote: > On 12/28/2020 1:33 PM, Luc Van Oostenryck wrote: > > On Mon, Dec 28, 2020 at 05:18:50PM +0000, Ramsay Jones wrote: > >> > >> Patch #16 also looks good, but I would need to study it a bit more > >> than I have time available to be totally happy. It does not seem > >> to be handling the 'lowering' of 'odd bit-sized' symbols created in > >> the previous patch (to answer my own question), so I would have to > >> apply the patches (or fetch the above branch) to study some more. > > > > Well, patch #16 doesn't contain the lowering, it kinda just enables > > the last bits to support (without out-of-bound access) packed structures > > including packed bitfields. > > > > What does the 'lowering' gain us? Or, in other words, what is still > missing after this series? As a static checker, nothing is missing and the series is now mainlined. The 'lowering' only matters if you want to somehow translate the instructions used in the IR (Intermediate Representation) into instructions for a more concrete machine, because now there are things like: 'do a load of a 5-byte word'. -- Luc
On 1/5/2021 12:39 PM, Luc Van Oostenryck wrote: > On Tue, Jan 05, 2021 at 09:56:05AM -0800, Jacob Keller wrote: >> On 12/28/2020 1:33 PM, Luc Van Oostenryck wrote: >>> On Mon, Dec 28, 2020 at 05:18:50PM +0000, Ramsay Jones wrote: >>>> >>>> Patch #16 also looks good, but I would need to study it a bit more >>>> than I have time available to be totally happy. It does not seem >>>> to be handling the 'lowering' of 'odd bit-sized' symbols created in >>>> the previous patch (to answer my own question), so I would have to >>>> apply the patches (or fetch the above branch) to study some more. >>> >>> Well, patch #16 doesn't contain the lowering, it kinda just enables >>> the last bits to support (without out-of-bound access) packed structures >>> including packed bitfields. >>> >> >> What does the 'lowering' gain us? Or, in other words, what is still >> missing after this series? > > As a static checker, nothing is missing and the series is now mainlined. > > The 'lowering' only matters if you want to somehow translate the > instructions used in the IR (Intermediate Representation) into > instructions for a more concrete machine, because now there are things > like: 'do a load of a 5-byte word'. > > -- Luc > Ok thanks, that makes sense. - Jake
During parsing, Sparse recognizes the attribute 'packed' but this attribute is otherwise ignored for several reasons: 1) the attribute in 'struct __attr { ... }' is wrongly handled as belonging to the whole declaration but it should belong to the type, 2) the attribute in 'struct <name> { ... } __attr;' is simply ignored, 3) the layout of packed bitfields need special care. This series contains 2 parts: 1) handling of type attributes 2) correct layout of packed structs, including packed bitfields. This series is also available for review and testing at: git://git.kernel.org/pub/scm/devel/sparse/sparse-dev.git packed-v2 Changes since v1: * fix layout of packed bitfields Luc Van Oostenryck (16): add testcases for dubious enum values add testcases for exotic enum values add testcases for enum attributes add testcases for type attributes add testcases for packed structures add testcases for packed bitfields apply_ctype: use self-explanatory argument name apply_ctype: reverse the order of arguments apply_ctype: move up its declaration struct-attr: prepare to handle attributes at the end of struct definitions (1) struct-attr: prepare to handle attributes at the end of struct definitions (2) struct-attr: prepare to handle attributes at the end of struct definitions (3) struct-attr: fix type attribute like 'struct __attr { ... }' struct-attr: fix: do not ignore struct/union/enum type attributes packed: no out-of-bound access of packed bitfields packed: add support for __packed struct linearize.c | 13 +++++- parse.c | 71 ++++++++++++++++--------------- symbol.c | 12 ++++-- symbol.h | 2 + validation/enum-type-dubious.c | 18 ++++++++ validation/enum-type-exotic.c | 28 ++++++++++++ validation/packed-bitfield0.c | 66 ++++++++++++++++++++++++++++ validation/packed-bitfield1.c | 27 ++++++++++++ validation/packed-bitfield2.c | 15 +++++++ validation/packed-bitfield3.c | 28 ++++++++++++ validation/packed-bitfield4.c | 18 ++++++++ validation/packed-bitfield5.c | 20 +++++++++ validation/packed-deref0.c | 23 ++++++++++ validation/packed-struct.c | 32 ++++++++++++++ validation/parsing/enum-attr.c | 29 +++++++++++++ validation/type-attribute-align.c | 19 +++++++++ validation/type-attribute-as.c | 33 ++++++++++++++ validation/type-attribute-mod.c | 21 +++++++++ validation/type-attribute-qual.c | 12 ++++++ 19 files changed, 448 insertions(+), 39 deletions(-) create mode 100644 validation/enum-type-dubious.c create mode 100644 validation/enum-type-exotic.c create mode 100644 validation/packed-bitfield0.c create mode 100644 validation/packed-bitfield1.c create mode 100644 validation/packed-bitfield2.c create mode 100644 validation/packed-bitfield3.c create mode 100644 validation/packed-bitfield4.c create mode 100644 validation/packed-bitfield5.c create mode 100644 validation/packed-deref0.c create mode 100644 validation/packed-struct.c create mode 100644 validation/parsing/enum-attr.c create mode 100644 validation/type-attribute-align.c create mode 100644 validation/type-attribute-as.c create mode 100644 validation/type-attribute-mod.c create mode 100644 validation/type-attribute-qual.c base-commit: 1b896707d95982c7c9cdd5cd0ab4afd80f766a94 Cc: Jacob Keller <jacob.e.keller@intel.com>