linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/16] support __packed struct
@ 2020-12-31 10:10 Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 01/16] add testcases for dubious enum values Luc Van Oostenryck
                   ` (16 more replies)
  0 siblings, 17 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

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-v3


Changes since v2 (all thanks to Ramsay Jones):
* correct several typos
* add an explanation for one of the test
* avoid sizeof() in tests where the size is constrained to some fixed value
* remove a redundency in a test (struct sb identical to struct sa)
* reformulate the commit message for apply_ctype()'s arguments reversal
* tweak the not-so-1-to-1 code movement
* fix the truncated commit message of the last patch

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

 Documentation/TODO.md             |  3 --
 linearize.c                       | 13 +++++-
 parse.c                           | 78 ++++++++++++++++---------------
 symbol.c                          | 12 +++--
 symbol.h                          |  2 +
 validation/enum-type-dubious.c    | 18 +++++++
 validation/enum-type-exotic.c     | 28 +++++++++++
 validation/packed-bitfield0.c     | 58 +++++++++++++++++++++++
 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  | 15 ++++++
 20 files changed, 447 insertions(+), 45 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

-- 
2.29.2


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 01/16] add testcases for dubious enum values
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 02/16] add testcases for exotic " Luc Van Oostenryck
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

sparse accept any type of integral value for enumerators
but address constants are also accepted, which is 'strange'.

Add a testcase for such 'enums'.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/enum-type-dubious.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 validation/enum-type-dubious.c

diff --git a/validation/enum-type-dubious.c b/validation/enum-type-dubious.c
new file mode 100644
index 000000000000..f2cb39fcdbff
--- /dev/null
+++ b/validation/enum-type-dubious.c
@@ -0,0 +1,18 @@
+enum foobar {
+	FOO = (void*)0,
+	BAR = (void*)1,
+	BAZ = (int*)0,
+	QUX = (int*)123,
+};
+
+/*
+ * check-name: enum-type-dubious
+ * check-known-to-fail
+ *
+ * check-error-start
+validation/enum-type-dubious.c:2:8: error: enumerator value for 'FOO' is not an integer constant
+validation/enum-type-dubious.c:3:8: error: enumerator value for 'BAR' is not an integer constant
+validation/enum-type-dubious.c:4:8: error: enumerator value for 'BAZ' is not an integer constant
+validation/enum-type-dubious.c:5:8: error: enumerator value for 'QUX' is not an integer constant
+ * check-error-end
+ */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 02/16] add testcases for exotic enum values
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 01/16] add testcases for dubious enum values Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 03/16] add testcases for enum attributes Luc Van Oostenryck
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

There is more than one complexity in the evaluation of enums.

Add a test for enums with 'exotic' values not covered in other tests.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/enum-type-exotic.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 validation/enum-type-exotic.c

diff --git a/validation/enum-type-exotic.c b/validation/enum-type-exotic.c
new file mode 100644
index 000000000000..a17ca0ad48ef
--- /dev/null
+++ b/validation/enum-type-exotic.c
@@ -0,0 +1,28 @@
+enum foobar {
+        C = (unsigned char)0,
+        L = 1L,
+};
+
+unsigned int foo(void);
+unsigned int foo(void)
+{
+#ifdef __CHECKER__
+	_Static_assert([typeof(C)] == [enum foobar], "enum type");
+	_Static_assert([typeof(C)] != [unsigned char], "char type");
+#endif
+
+	typeof(C) v = ~0;
+	return v;
+}
+
+/*
+ * check-name: enum-type-exotic
+ * check-description:
+ *	GCC type's for C is 'int' or maybe 'unsigned int'
+ *	but certainly not 'unsigned char' like here.
+ * check-command: test-linearize -Wno-decl $file
+ * check-known-to-fail
+ *
+ * check-output-ignore
+ * check-output-excludes: ret\\.32 *\\$255
+ */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 03/16] add testcases for enum attributes
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 01/16] add testcases for dubious enum values Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 02/16] add testcases for exotic " Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 04/16] add testcases for type attributes Luc Van Oostenryck
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/parsing/enum-attr.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 validation/parsing/enum-attr.c

diff --git a/validation/parsing/enum-attr.c b/validation/parsing/enum-attr.c
new file mode 100644
index 000000000000..a962d8b417af
--- /dev/null
+++ b/validation/parsing/enum-attr.c
@@ -0,0 +1,29 @@
+#define __attr __attribute__((deprecated))
+
+enum {
+	old __attr,
+	cur __attr = 42,
+	new,
+};
+
+enum odd {
+	odd = __attr 33,
+};
+
+enum bad {
+	bad = 43 __attr,
+};
+
+/*
+ * check-name: enum-attr
+ *
+ * check-error-start
+parsing/enum-attr.c:10:15: error: typename in expression
+parsing/enum-attr.c:10:15: error: undefined identifier '__attribute__'
+parsing/enum-attr.c:10:15: error: bad constant expression type
+parsing/enum-attr.c:10:22: error: Expected } at end of struct-union-enum-specifier
+parsing/enum-attr.c:10:22: error: got 33
+parsing/enum-attr.c:14:18: error: Expected } at end of struct-union-enum-specifier
+parsing/enum-attr.c:14:18: error: got __attribute__
+ * check-error-end
+ */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 04/16] add testcases for type attributes
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (2 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 03/16] add testcases for enum attributes Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 05/16] add testcases for packed structures Luc Van Oostenryck
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

Currently, type attributes are not handled correctly.

Add some testcases for them.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/type-attribute-align.c | 20 ++++++++++++++++++
 validation/type-attribute-as.c    | 34 +++++++++++++++++++++++++++++++
 validation/type-attribute-mod.c   | 22 ++++++++++++++++++++
 validation/type-attribute-qual.c  | 15 ++++++++++++++
 4 files changed, 91 insertions(+)
 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

diff --git a/validation/type-attribute-align.c b/validation/type-attribute-align.c
new file mode 100644
index 000000000000..d9358bff8327
--- /dev/null
+++ b/validation/type-attribute-align.c
@@ -0,0 +1,20 @@
+#define __aligned(N)	__attribute__((aligned(N)))
+#define alignof(X)	__alignof__(X)
+
+struct s {
+	short a, b, c;
+} __aligned(2*sizeof(short));
+
+static int fs(void) { return  sizeof(struct s); }
+static int fa(void) { return alignof(struct s); }
+
+void main(void)
+{
+	_Static_assert( sizeof(struct s) == 4 * sizeof(short), "size");
+	_Static_assert(alignof(struct s) == 2 * sizeof(short), "alignment");
+}
+
+/*
+ * check-name: type-attribute-align
+ * check-known-to-fail
+ */
diff --git a/validation/type-attribute-as.c b/validation/type-attribute-as.c
new file mode 100644
index 000000000000..b40b4e7dddf5
--- /dev/null
+++ b/validation/type-attribute-as.c
@@ -0,0 +1,34 @@
+#define	__as		__attribute__((address_space(__as)))
+
+struct s {
+	int i;
+} __as;
+
+
+extern void use0(void *);
+extern void use1(void __as *);
+
+void main(void)
+{
+	struct s s;
+	int i;
+
+	use0(&s);	// KO
+	use0(&i);	// OK
+	use1(&s);	// OK
+	use1(&i);	// KO
+}
+
+/*
+ * check-name: type-attribute-as
+ * check-known-to-fail
+ *
+ * check-error-start
+type-attribute-as.c:16:15: warning: incorrect type in argument 1 (different address spaces)
+type-attribute-as.c:16:15:    expected void *
+type-attribute-as.c:16:15:    got struct s __as *
+type-attribute-as.c:19:15: warning: incorrect type in argument 1 (different address spaces)
+type-attribute-as.c:19:15:    expected void __as *
+type-attribute-as.c:19:15:    got int *
+ * check-error-end
+ */
diff --git a/validation/type-attribute-mod.c b/validation/type-attribute-mod.c
new file mode 100644
index 000000000000..0e7b166a4aec
--- /dev/null
+++ b/validation/type-attribute-mod.c
@@ -0,0 +1,22 @@
+#define	__noderef	__attribute__((noderef))
+
+struct s {
+	int i;
+} __noderef;
+
+
+void main(void)
+{
+	struct s s;
+
+	s.i = 0;
+}
+
+/*
+ * check-name: type-attribute-mod
+ * check-known-to-fail
+ *
+ * check-error-start
+type-attribute-mod.c:12:9: warning: dereference of noderef expression
+ * check-error-end
+ */
diff --git a/validation/type-attribute-qual.c b/validation/type-attribute-qual.c
new file mode 100644
index 000000000000..62d8b98ee3df
--- /dev/null
+++ b/validation/type-attribute-qual.c
@@ -0,0 +1,15 @@
+static const struct s {
+	int x;
+} map[2];
+
+static void foo(struct s *p, int v)
+{
+	p->x += v;
+}
+
+/*
+ * check-name: type-attribute-qual
+ * check-description: When declaring a type and a variable in the same
+ *	declaration, ensure that type qualifiers apply to the variable
+ *	and not to the type.
+ */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 05/16] add testcases for packed structures
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (3 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 04/16] add testcases for type attributes Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 06/16] add testcases for packed bitfields Luc Van Oostenryck
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

Currently, packed structs are not handled correctly.

Add some testcases for them.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/packed-deref0.c | 24 ++++++++++++++++++++++++
 validation/packed-struct.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)
 create mode 100644 validation/packed-deref0.c
 create mode 100644 validation/packed-struct.c

diff --git a/validation/packed-deref0.c b/validation/packed-deref0.c
new file mode 100644
index 000000000000..865ad68a4f37
--- /dev/null
+++ b/validation/packed-deref0.c
@@ -0,0 +1,24 @@
+#define	__packed	__attribute__((packed))
+
+typedef struct {
+	__INT8_TYPE__	a;
+	__INT16_TYPE__	b;
+	__INT32_TYPE__	c;
+} __packed obj_t;
+
+_Static_assert(sizeof(obj_t) == 7, "sizeof packed struct");
+
+static void foo(obj_t *ptr, int val)
+{
+	ptr->c = val;
+}
+
+static void bar(obj_t o)
+{
+	foo(&o, 0);
+}
+
+/*
+ * check-name: packed-deref0
+ * check-known-to-fail
+ */
diff --git a/validation/packed-struct.c b/validation/packed-struct.c
new file mode 100644
index 000000000000..e21d11538639
--- /dev/null
+++ b/validation/packed-struct.c
@@ -0,0 +1,33 @@
+#define __packed __attribute__((packed))
+
+typedef unsigned char   u8;
+typedef __UINT16_TYPE__ u16;
+typedef __UINT32_TYPE__ u32;
+typedef __UINT64_TYPE__ u64;
+
+struct a {
+	u8 a;
+	u8 b;
+	u16 c;
+} __packed;
+_Static_assert(__alignof(struct a) == 1, "align struct");
+_Static_assert(   sizeof(struct a) == 4, " size struct");
+
+struct b {
+	u32	a;
+	u32	b;
+} __packed;
+_Static_assert(__alignof(struct b) == 1, "align struct");
+_Static_assert(   sizeof(struct b) == 8, "size struct");
+
+struct c {
+	u16	a;
+	u32	b;
+} __packed;
+_Static_assert(__alignof(struct c) == 1, "align struct");
+_Static_assert(   sizeof(struct c) == 6, "size struct");
+
+/*
+ * check-name: packed-struct
+ * check-known-to-fail
+ */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 06/16] add testcases for packed bitfields
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (4 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 05/16] add testcases for packed structures Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 07/16] apply_ctype: use self-explanatory argument name Luc Van Oostenryck
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

Currently, packed bitfields are not handled correctly.

Add some testcases for them.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/packed-bitfield0.c | 59 +++++++++++++++++++++++++++++++++++
 validation/packed-bitfield1.c | 28 +++++++++++++++++
 validation/packed-bitfield2.c | 16 ++++++++++
 validation/packed-bitfield3.c | 29 +++++++++++++++++
 validation/packed-bitfield4.c | 19 +++++++++++
 validation/packed-bitfield5.c | 21 +++++++++++++
 6 files changed, 172 insertions(+)
 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

diff --git a/validation/packed-bitfield0.c b/validation/packed-bitfield0.c
new file mode 100644
index 000000000000..f84e7b904a82
--- /dev/null
+++ b/validation/packed-bitfield0.c
@@ -0,0 +1,59 @@
+#define alignof(X)	__alignof__(X)
+#define __packed	__attribute__((packed))
+
+struct sa {
+	int a:7;
+	int c:10;
+	int b:2;
+} __packed;
+_Static_assert(alignof(struct sa) == 1, "alignof(struct sa)");
+_Static_assert( sizeof(struct sa) == 3,  "sizeof(struct sa)");
+
+
+static int get_size(void)
+{
+	return sizeof(struct sa);
+}
+
+static void chk_align(struct sa sa, struct sa *p)
+{
+	_Static_assert(alignof(sa) == 1, "alignof(sa)");
+	_Static_assert(alignof(*p) == 1, "alignof(*p)");
+}
+
+static int fp0(struct sa *sa)
+{
+	return sa->c;
+}
+
+static int fpx(struct sa *sa, int idx)
+{
+	return sa[idx].c;
+}
+
+static int fglobal(void)
+{
+	extern struct sa g;
+	return g.c;
+}
+
+static struct sa l;
+static int flocal(void)
+{
+	return l.c;
+}
+
+
+int main(void)
+{
+	extern void fun(struct sa *);
+	struct sa sa = { 0 };
+
+	fun(&sa);
+	return 0;
+}
+
+/*
+ * check-name: packed-bitfield0
+ * check-known-to-fail
+ */
diff --git a/validation/packed-bitfield1.c b/validation/packed-bitfield1.c
new file mode 100644
index 000000000000..208a3dc5127c
--- /dev/null
+++ b/validation/packed-bitfield1.c
@@ -0,0 +1,28 @@
+#define __packed	__attribute__((packed))
+
+struct s {
+	unsigned int f0:1;
+	unsigned int f1:1;
+	unsigned int pad:6;
+} __packed;
+_Static_assert(sizeof(struct s) == 1,  "sizeof(struct s)");
+
+extern struct s g;
+
+static int foo(struct s *ptr)
+{
+	int f = 0;
+
+	f += g.f0;
+	f += g.f1;
+
+	f += ptr->f0;
+	f += ptr->f1;
+
+	return f;
+}
+
+/*
+ * check-name: packed-bitfield1
+ * check-known-to-fail
+ */
diff --git a/validation/packed-bitfield2.c b/validation/packed-bitfield2.c
new file mode 100644
index 000000000000..4587ebec5c90
--- /dev/null
+++ b/validation/packed-bitfield2.c
@@ -0,0 +1,16 @@
+struct bf2 {
+	unsigned p1:2;
+	unsigned i1:32;
+	unsigned p2:2;
+	unsigned s9:9;
+	unsigned s9:9;
+	unsigned s9:9;
+	unsigned b1:1;
+} __attribute__((packed));
+
+_Static_assert(sizeof(struct bf2) == 8);
+
+/*
+ * check-name: packed-bitfield2
+ * check-known-to-fail
+ */
diff --git a/validation/packed-bitfield3.c b/validation/packed-bitfield3.c
new file mode 100644
index 000000000000..c06e7c41cbcd
--- /dev/null
+++ b/validation/packed-bitfield3.c
@@ -0,0 +1,29 @@
+#define __packed __attribute__((packed))
+
+typedef unsigned char   u8;
+typedef __UINT16_TYPE__ u16;
+typedef __UINT32_TYPE__ u32;
+typedef __UINT64_TYPE__ u64;
+
+struct b {
+	u32	a:1;
+	u32	b:2;
+	u32	c:4;
+	u32	d:8;
+	u32	e:16;
+} __packed;
+_Static_assert(__alignof(struct b) == 1);
+_Static_assert(   sizeof(struct b) == 4);
+
+struct c {
+	u8	a;
+	u8	b;
+	u64	c:48;
+} __packed;
+_Static_assert(__alignof(struct c) == 1);
+_Static_assert(   sizeof(struct c) == 8);
+
+/*
+ * check-name: packed-bitfield3
+ * check-known-to-fail
+ */
diff --git a/validation/packed-bitfield4.c b/validation/packed-bitfield4.c
new file mode 100644
index 000000000000..0342b2414b6e
--- /dev/null
+++ b/validation/packed-bitfield4.c
@@ -0,0 +1,19 @@
+#define __packed __attribute__((packed))
+
+typedef __UINT32_TYPE__ u32;
+
+struct s {
+	u32	f:24;
+} __packed;
+_Static_assert(sizeof(struct s) == 3);
+
+static int ld(struct s *s)
+{
+	return s->f;
+}
+
+/*
+ * check-name: packed-bitfield4
+ * check-description: Is check_access() OK with short packed bitfields?
+ * check-known-to-fail
+ */
diff --git a/validation/packed-bitfield5.c b/validation/packed-bitfield5.c
new file mode 100644
index 000000000000..8f44d4c2c277
--- /dev/null
+++ b/validation/packed-bitfield5.c
@@ -0,0 +1,21 @@
+#define __packed __attribute__((packed))
+
+typedef __UINT32_TYPE__ u32;
+
+struct s {
+	u32	a:5;
+	u32	f:30;
+	u32	z:5;
+} __packed;
+_Static_assert(sizeof(struct s) == 5);
+
+static int ld(struct s *s)
+{
+	return s->f;
+}
+
+/*
+ * check-name: packed-bitfield5
+ * check-description: is check_access() OK with 'overlapping' packed bitfields?
+ * check-known-to-fail
+ */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 07/16] apply_ctype: use self-explanatory argument name
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (5 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 06/16] add testcases for packed bitfields Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 08/16] apply_ctype: reverse the order of arguments Luc Van Oostenryck
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

apply_ctype() is used to copy/apply things like modifiers
and address space from one type to another one.
But the names used for the two types are: 'ctype' & 'thistype'
which doesn't help at all to know which one is the 'source' type
and which one is the 'destination' type.

Change this by renaming these arguments to 'src' & 'dst'.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/parse.c b/parse.c
index 0b2685707b82..402214212d77 100644
--- a/parse.c
+++ b/parse.c
@@ -1043,7 +1043,7 @@ static struct token *enum_specifier(struct token *token, struct symbol *sym, str
 	return ret;
 }
 
-static void apply_ctype(struct position pos, struct ctype *thistype, struct ctype *ctype);
+static void apply_ctype(struct position pos, struct ctype *src, struct ctype *dst);
 
 static struct token *typeof_specifier(struct token *token, struct symbol *sym, struct decl_state *ctx)
 {
@@ -1427,24 +1427,24 @@ static struct token *generic_qualifier(struct token *next, struct symbol *sym, s
 	return next;
 }
 
-static void apply_ctype(struct position pos, struct ctype *thistype, struct ctype *ctype)
+static void apply_ctype(struct position pos, struct ctype *src, struct ctype *dst)
 {
-	unsigned long mod = thistype->modifiers;
+	unsigned long mod = src->modifiers;
 
 	if (mod)
-		apply_qualifier(&pos, ctype, mod);
+		apply_qualifier(&pos, dst, mod);
 
 	/* Context */
-	concat_ptr_list((struct ptr_list *)thistype->contexts,
-	                (struct ptr_list **)&ctype->contexts);
+	concat_ptr_list((struct ptr_list *)src->contexts,
+	                (struct ptr_list **)&dst->contexts);
 
 	/* Alignment */
-	if (thistype->alignment > ctype->alignment)
-		ctype->alignment = thistype->alignment;
+	if (src->alignment > dst->alignment)
+		dst->alignment = src->alignment;
 
 	/* Address space */
-	if (thistype->as)
-		ctype->as = thistype->as;
+	if (src->as)
+		dst->as = src->as;
 }
 
 static void specifier_conflict(struct position pos, int what, struct ident *new)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 08/16] apply_ctype: reverse the order of arguments
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (6 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 07/16] apply_ctype: use self-explanatory argument name Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 09/16] apply_ctype: move up its declaration Luc Van Oostenryck
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

apply_ctype()'s argument order confuse me endlessly as I'm much more
used to have the destination first and the source next (the so called
'assignment order' used for assignments but also in memcpy() and in
many sparse or library functions).

So, change the argument order of apply_ctype() to mimic the order
of memcpy()/assignment, to hopefully reduce my confusion.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/parse.c b/parse.c
index 402214212d77..f106444f75d8 100644
--- a/parse.c
+++ b/parse.c
@@ -1043,7 +1043,7 @@ static struct token *enum_specifier(struct token *token, struct symbol *sym, str
 	return ret;
 }
 
-static void apply_ctype(struct position pos, struct ctype *src, struct ctype *dst);
+static void apply_ctype(struct position pos, struct ctype *dst, struct ctype *src);
 
 static struct token *typeof_specifier(struct token *token, struct symbol *sym, struct decl_state *ctx)
 {
@@ -1056,7 +1056,7 @@ static struct token *typeof_specifier(struct token *token, struct symbol *sym, s
 		struct symbol *sym;
 		token = typename(token->next, &sym, NULL);
 		ctx->ctype.base_type = sym->ctype.base_type;
-		apply_ctype(token->pos, &sym->ctype, &ctx->ctype);
+		apply_ctype(token->pos, &ctx->ctype, &sym->ctype);
 	} else {
 		struct symbol *typeof_sym = alloc_symbol(token->pos, SYM_TYPEOF);
 		token = parse_expression(token->next, &typeof_sym->initializer);
@@ -1427,7 +1427,7 @@ static struct token *generic_qualifier(struct token *next, struct symbol *sym, s
 	return next;
 }
 
-static void apply_ctype(struct position pos, struct ctype *src, struct ctype *dst)
+static void apply_ctype(struct position pos, struct ctype *dst, struct ctype *src)
 {
 	unsigned long mod = src->modifiers;
 
@@ -1529,7 +1529,7 @@ static struct token *declaration_specifiers(struct token *token, struct decl_sta
 				break;
 			seen |= Set_S | Set_T;
 			ctx->ctype.base_type = s->ctype.base_type;
-			apply_ctype(token->pos, &s->ctype, &ctx->ctype);
+			apply_ctype(token->pos, &ctx->ctype, &s->ctype);
 			token = token->next;
 			continue;
 		}
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 09/16] apply_ctype: move up its declaration
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (7 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 08/16] apply_ctype: reverse the order of arguments Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 10/16] struct-attr: prepare to handle attributes at the end of struct definitions (1) Luc Van Oostenryck
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

apply_ctype() will be needed earlier in the code.
So, move it's prototype up.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/parse.c b/parse.c
index f106444f75d8..d6343f0e48bf 100644
--- a/parse.c
+++ b/parse.c
@@ -669,6 +669,8 @@ struct statement *alloc_statement(struct position pos, int type)
 
 static struct token *struct_declaration_list(struct token *token, struct symbol_list **list);
 
+static void apply_ctype(struct position pos, struct ctype *dst, struct ctype *src);
+
 static void apply_modifiers(struct position pos, struct decl_state *ctx)
 {
 	struct symbol *ctype;
@@ -1043,8 +1045,6 @@ static struct token *enum_specifier(struct token *token, struct symbol *sym, str
 	return ret;
 }
 
-static void apply_ctype(struct position pos, struct ctype *dst, struct ctype *src);
-
 static struct token *typeof_specifier(struct token *token, struct symbol *sym, struct decl_state *ctx)
 {
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 10/16] struct-attr: prepare to handle attributes at the end of struct definitions (1)
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (8 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 09/16] apply_ctype: move up its declaration Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 11/16] struct-attr: prepare to handle attributes at the end of struct definitions (2) Luc Van Oostenryck
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

Type attributes for struct can be placed either just after the
keyword 'struct' or after the '}' ending its definition but this
later case is currently ignored.

Prepare the handling of this by factoring the code common to both
cases in a single place.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c                        | 10 +++++-----
 validation/parsing/enum-attr.c |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/parse.c b/parse.c
index d6343f0e48bf..41d3eb1ff4f8 100644
--- a/parse.c
+++ b/parse.c
@@ -745,12 +745,10 @@ static struct token *struct_union_enum_specifier(enum type type,
 			if (sym->symbol_list)
 				error_die(token->pos, "redefinition of %s", show_typename (sym));
 			sym->pos = *repos;
-			token = parse(token->next, sym);
-			token = expect(token, '}', "at end of struct-union-enum-specifier");
 
 			// Mark the structure as needing re-examination
 			sym->examined = 0;
-			sym->endpos = token->pos;
+			goto end;
 		}
 		return token;
 	}
@@ -764,9 +762,11 @@ static struct token *struct_union_enum_specifier(enum type type,
 
 	sym = alloc_symbol(token->pos, type);
 	set_current_scope(sym);		// used by dissect
-	token = parse(token->next, sym);
 	ctx->ctype.base_type = sym;
-	token =  expect(token, '}', "at end of specifier");
+end:
+	token = parse(token->next, sym);
+	token = expect(token, '}', "at end of specifier");
+
 	sym->endpos = token->pos;
 
 	return token;
diff --git a/validation/parsing/enum-attr.c b/validation/parsing/enum-attr.c
index a962d8b417af..8d851a162135 100644
--- a/validation/parsing/enum-attr.c
+++ b/validation/parsing/enum-attr.c
@@ -21,9 +21,9 @@ enum bad {
 parsing/enum-attr.c:10:15: error: typename in expression
 parsing/enum-attr.c:10:15: error: undefined identifier '__attribute__'
 parsing/enum-attr.c:10:15: error: bad constant expression type
-parsing/enum-attr.c:10:22: error: Expected } at end of struct-union-enum-specifier
+parsing/enum-attr.c:10:22: error: Expected } at end of specifier
 parsing/enum-attr.c:10:22: error: got 33
-parsing/enum-attr.c:14:18: error: Expected } at end of struct-union-enum-specifier
+parsing/enum-attr.c:14:18: error: Expected } at end of specifier
 parsing/enum-attr.c:14:18: error: got __attribute__
  * check-error-end
  */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 11/16] struct-attr: prepare to handle attributes at the end of struct definitions (2)
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (9 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 10/16] struct-attr: prepare to handle attributes at the end of struct definitions (1) Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 12/16] struct-attr: prepare to handle attributes at the end of struct definitions (3) Luc Van Oostenryck
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

Type attributes for struct can be placed either just after the
keyword 'struct' or after the '}' ending its definition but this
later case is currently ignored.

Prepare the handling of this by restructuring the code handling
struct specifiers, namely inverting the condition so that the
function can return early to make next patch's job easier.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/parse.c b/parse.c
index 41d3eb1ff4f8..663f2141dbf4 100644
--- a/parse.c
+++ b/parse.c
@@ -738,19 +738,19 @@ static struct token *struct_union_enum_specifier(enum type type,
 		ctx->ctype.base_type = sym;
 		repos = &token->pos;
 		token = token->next;
-		if (match_op(token, '{')) {
-			// The following test is actually wrong for empty
-			// structs, but (1) they are not C99, (2) gcc does
-			// the same thing, and (3) it's easier.
-			if (sym->symbol_list)
-				error_die(token->pos, "redefinition of %s", show_typename (sym));
-			sym->pos = *repos;
-
-			// Mark the structure as needing re-examination
-			sym->examined = 0;
-			goto end;
-		}
-		return token;
+		if (!match_op(token, '{'))
+			return token;
+
+		// The following test is actually wrong for empty
+		// structs, but (1) they are not C99, (2) gcc does
+		// the same thing, and (3) it's easier.
+		if (sym->symbol_list)
+			error_die(token->pos, "redefinition of %s", show_typename (sym));
+		sym->pos = *repos;
+
+		// Mark the structure as needing re-examination
+		sym->examined = 0;
+		goto end;
 	}
 
 	// private struct/union/enum type
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 12/16] struct-attr: prepare to handle attributes at the end of struct definitions (3)
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (10 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 11/16] struct-attr: prepare to handle attributes at the end of struct definitions (2) Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 13/16] struct-attr: fix type attribute like 'struct __attr { ... }' Luc Van Oostenryck
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

Type attributes for struct can be placed either just after the
keyword 'struct' or after the '}' ending its definition but this
later case is currently ignored.

Prepare the handling of this by having the 3 following cases in sequence:
1) a tag is present
2) no tag present but is followed by an opening brace
3) neither of these, so it's an error.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/parse.c b/parse.c
index 663f2141dbf4..69bfc2e0fe57 100644
--- a/parse.c
+++ b/parse.c
@@ -750,20 +750,17 @@ static struct token *struct_union_enum_specifier(enum type type,
 
 		// Mark the structure as needing re-examination
 		sym->examined = 0;
-		goto end;
-	}
-
-	// private struct/union/enum type
-	if (!match_op(token, '{')) {
+	} else if (match_op(token, '{')) {
+		// private struct/union/enum type
+		sym = alloc_symbol(token->pos, type);
+		set_current_scope(sym);		// used by dissect
+		ctx->ctype.base_type = sym;
+	} else {
 		sparse_error(token->pos, "expected declaration");
 		ctx->ctype.base_type = &bad_ctype;
 		return token;
 	}
 
-	sym = alloc_symbol(token->pos, type);
-	set_current_scope(sym);		// used by dissect
-	ctx->ctype.base_type = sym;
-end:
 	token = parse(token->next, sym);
 	token = expect(token, '}', "at end of specifier");
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 13/16] struct-attr: fix type attribute like 'struct __attr { ... }'
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (11 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 12/16] struct-attr: prepare to handle attributes at the end of struct definitions (3) Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 14/16] struct-attr: fix: do not ignore struct/union/enum type attributes Luc Van Oostenryck
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

In a declaration like:
	struct <some attribute> { ... }
the attribute belong to the type but is currently handled as belonging
to the whole declaration.

Fix this by handling such attributes in a local 'decl_state' and
applying them once the closing '}' is reached.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index 69bfc2e0fe57..b38615b8d61d 100644
--- a/parse.c
+++ b/parse.c
@@ -719,10 +719,11 @@ static struct token *struct_union_enum_specifier(enum type type,
 	struct token *token, struct decl_state *ctx,
 	struct token *(*parse)(struct token *, struct symbol *))
 {
+	struct decl_state attr = { };
 	struct symbol *sym;
 	struct position *repos;
 
-	token = handle_attributes(token, ctx);
+	token = handle_attributes(token, &attr);
 	if (token_type(token) == TOKEN_IDENT) {
 		sym = lookup_symbol(token->ident, NS_STRUCT);
 		if (!sym ||
@@ -763,6 +764,7 @@ static struct token *struct_union_enum_specifier(enum type type,
 
 	token = parse(token->next, sym);
 	token = expect(token, '}', "at end of specifier");
+	apply_ctype(token->pos, &sym->ctype, &attr.ctype);
 
 	sym->endpos = token->pos;
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 14/16] struct-attr: fix: do not ignore struct/union/enum type attributes
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (12 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 13/16] struct-attr: fix type attribute like 'struct __attr { ... }' Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 15/16] packed: no out-of-bound access of packed bitfields Luc Van Oostenryck
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

GCC's syntax for type attributes is specified as:
    An attribute specifier list may appear as part of a struct,
    union or enum specifier. It may go either immediately after
    the struct, union or enum keyword, or after the closing brace.
    The former syntax is preferred. Where attribute specifiers
    follow the closing brace, they are considered to relate to
    the structure, union or enumerated type defined, not to any
    enclosing declaration the type specifier appears in, and the type
    defined is not complete until after the attribute specifiers.
In the section about type attributes, it's also said:
    You may specify type attributes in an enum, struct or union type
    declaration or definition by placing them immediately after the
    struct, union or enum keyword. A less preferred syntax is to
    place them just past the closing curly brace of the definition.

So, while placing the attribute after the closing curly is not
preferred, it is cleary legal (and it seems to be much more popular
than placing them just after the struct, union or enum keyword).

However, currently sparse doesn't handle this correctly:
- these attributes are parsed in declaration_specifiers() and
  added to the current decl_state
- when the ';' ending the type declaration is reached, the plain
  struct/union/enum is used and the content of the decl_state is
  simply ignored.
- if the declaration is for a variable, then those attributes
  are assigned to the variable (but not to the type).

Fix this by calling handle_attribute() once we have reached the
closing '}' of a struct/union/enum definition and applying these
attributes, if any, directly to the current base type.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c                           | 2 ++
 validation/packed-bitfield3.c     | 1 -
 validation/packed-bitfield4.c     | 1 -
 validation/type-attribute-align.c | 1 -
 validation/type-attribute-as.c    | 1 -
 validation/type-attribute-mod.c   | 1 -
 6 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/parse.c b/parse.c
index b38615b8d61d..338e525f46b9 100644
--- a/parse.c
+++ b/parse.c
@@ -764,6 +764,8 @@ static struct token *struct_union_enum_specifier(enum type type,
 
 	token = parse(token->next, sym);
 	token = expect(token, '}', "at end of specifier");
+	attr.ctype.base_type = sym;
+	token = handle_attributes(token, &attr);
 	apply_ctype(token->pos, &sym->ctype, &attr.ctype);
 
 	sym->endpos = token->pos;
diff --git a/validation/packed-bitfield3.c b/validation/packed-bitfield3.c
index c06e7c41cbcd..13368c84469e 100644
--- a/validation/packed-bitfield3.c
+++ b/validation/packed-bitfield3.c
@@ -25,5 +25,4 @@ _Static_assert(   sizeof(struct c) == 8);
 
 /*
  * check-name: packed-bitfield3
- * check-known-to-fail
  */
diff --git a/validation/packed-bitfield4.c b/validation/packed-bitfield4.c
index 0342b2414b6e..1a956344f2d3 100644
--- a/validation/packed-bitfield4.c
+++ b/validation/packed-bitfield4.c
@@ -15,5 +15,4 @@ static int ld(struct s *s)
 /*
  * check-name: packed-bitfield4
  * check-description: Is check_access() OK with short packed bitfields?
- * check-known-to-fail
  */
diff --git a/validation/type-attribute-align.c b/validation/type-attribute-align.c
index d9358bff8327..5f3db14aceb1 100644
--- a/validation/type-attribute-align.c
+++ b/validation/type-attribute-align.c
@@ -16,5 +16,4 @@ void main(void)
 
 /*
  * check-name: type-attribute-align
- * check-known-to-fail
  */
diff --git a/validation/type-attribute-as.c b/validation/type-attribute-as.c
index b40b4e7dddf5..38f06b374e25 100644
--- a/validation/type-attribute-as.c
+++ b/validation/type-attribute-as.c
@@ -21,7 +21,6 @@ void main(void)
 
 /*
  * check-name: type-attribute-as
- * check-known-to-fail
  *
  * check-error-start
 type-attribute-as.c:16:15: warning: incorrect type in argument 1 (different address spaces)
diff --git a/validation/type-attribute-mod.c b/validation/type-attribute-mod.c
index 0e7b166a4aec..d55011dfa09a 100644
--- a/validation/type-attribute-mod.c
+++ b/validation/type-attribute-mod.c
@@ -14,7 +14,6 @@ void main(void)
 
 /*
  * check-name: type-attribute-mod
- * check-known-to-fail
  *
  * check-error-start
 type-attribute-mod.c:12:9: warning: dereference of noderef expression
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 15/16] packed: no out-of-bound access of packed bitfields
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (13 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 14/16] struct-attr: fix: do not ignore struct/union/enum type attributes Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 10:10 ` [PATCH v3 16/16] packed: add support for __packed struct Luc Van Oostenryck
  2020-12-31 15:30 ` [PATCH v3 00/16] support " Ramsay Jones
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

There is (at least) 2 ways by which packed bitfields doesn't
follow normal layout/access rules and as consequence can't (always)
be accessed the usual way (load the whole underlying word, then shift
and mask to isolate the bitfield).

At least two different cases are a concern:
1) there is no padding at the end of a bitfield sequence. For example,
   the following struct is only 3 bytes width:
	struct s {
		int f:24;
	} __packed;
   So, trying to access the bitfield by first doing a 32-bit load
   will create an out-of-bound access.

2) a bitfield smaller than one word may need more than one word to be
   accessed. For example, with the following struct
	struct {
		int a:5;
		int f:30;
		int z:5;
	} __packed;
   the bitfield 'f', while smaller than one 32-bit word, can't be accessed
   with a single 32-bit access.

At machine level, these bitfields should be accessed with several, possibly
smaller, loads and their corresponding values reconstructed from these,
making things much more complicated than for non-packed bitfields.

But at IR level, things can be a little more flexible and things can stay
simple by using sub-word or super-word accesses (until these need to
be lowered to be usable at machine level). In other words, the example here
can be safely accessed with respectively a 24-bit and a 40-bit load.
This is what is done in this patch.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 linearize.c | 13 +++++++++++--
 symbol.h    |  1 +
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/linearize.c b/linearize.c
index 0250c6bb17ef..e80715ab2458 100644
--- a/linearize.c
+++ b/linearize.c
@@ -977,8 +977,17 @@ static struct symbol *bitfield_base_type(struct symbol *sym)
 	if (sym) {
 		if (sym->type == SYM_NODE)
 			base = base->ctype.base_type;
-		if (base->type == SYM_BITFIELD)
-			return base->ctype.base_type;
+		if (base->type == SYM_BITFIELD) {
+			base = base->ctype.base_type;
+			if (sym->packed) {
+				int size = bits_to_bytes(sym->bit_offset + sym->bit_size);
+				sym = __alloc_symbol(0);
+				*sym = *base;
+				sym->bit_size = bytes_to_bits(size);
+				return sym;
+			}
+			return base;
+		}
 	}
 	return sym;
 }
diff --git a/symbol.h b/symbol.h
index 5c5a7f12affa..866d57522f49 100644
--- a/symbol.h
+++ b/symbol.h
@@ -192,6 +192,7 @@ struct symbol {
 					accessed:1,
 					builtin:1,
 					torename:1,
+					packed:1,
 					transparent_union:1;
 			int		rank:3;	// arithmetic's rank
 			struct expression *array_size;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 16/16] packed: add support for __packed struct
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (14 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 15/16] packed: no out-of-bound access of packed bitfields Luc Van Oostenryck
@ 2020-12-31 10:10 ` Luc Van Oostenryck
  2020-12-31 15:30 ` [PATCH v3 00/16] support " Ramsay Jones
  16 siblings, 0 replies; 18+ messages in thread
From: Luc Van Oostenryck @ 2020-12-31 10:10 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck

Now that the 'packed' attribute is parsed and propagated into
the type system, adapt the layout of structures.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 Documentation/TODO.md         |  3 ---
 parse.c                       |  5 ++++-
 symbol.c                      | 12 +++++++++---
 symbol.h                      |  1 +
 validation/packed-bitfield0.c |  1 -
 validation/packed-bitfield1.c |  1 -
 validation/packed-bitfield2.c |  1 -
 validation/packed-bitfield5.c |  1 -
 validation/packed-deref0.c    |  1 -
 validation/packed-struct.c    |  1 -
 10 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/Documentation/TODO.md b/Documentation/TODO.md
index 4dc9e63ab207..3f00bb1104d1 100644
--- a/Documentation/TODO.md
+++ b/Documentation/TODO.md
@@ -4,9 +4,6 @@ TODO
 Essential
 ---------
 * SSA is broken by simplify_loads() & branches rewriting/simplification
-* attributes of struct, union & enums are ignored (and maybe others too).
-  This requires correct support for __packed which itself needs partial
-  and unaligned loads & stores (wip)
 * add support for bitwise enums (wip)
 
 Documentation
diff --git a/parse.c b/parse.c
index 338e525f46b9..70be616c45ae 100644
--- a/parse.c
+++ b/parse.c
@@ -767,6 +767,7 @@ static struct token *struct_union_enum_specifier(enum type type,
 	attr.ctype.base_type = sym;
 	token = handle_attributes(token, &attr);
 	apply_ctype(token->pos, &sym->ctype, &attr.ctype);
+	sym->packed = attr.packed;
 
 	sym->endpos = token->pos;
 
@@ -1089,8 +1090,10 @@ static struct token *ignore_attribute(struct token *token, struct symbol *attr,
 
 static struct token *attribute_packed(struct token *token, struct symbol *attr, struct decl_state *ctx)
 {
-	if (!ctx->ctype.alignment)
+	if (!ctx->ctype.alignment) {
 		ctx->ctype.alignment = 1;
+		ctx->packed = 1;
+	}
 	return token;
 }
 
diff --git a/symbol.c b/symbol.c
index 1a083fb8432c..aa02c8c5ad80 100644
--- a/symbol.c
+++ b/symbol.c
@@ -88,6 +88,7 @@ struct struct_union_info {
 	unsigned long bit_size;
 	int align_size;
 	char has_flex_array;
+	bool packed;
 	struct symbol *flex_array;
 };
 
@@ -120,6 +121,7 @@ static int bitfield_base_size(struct symbol *sym)
 static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 {
 	unsigned long bit_size, align_bit_mask;
+	unsigned long alignment;
 	int base_size;
 
 	bit_size = info->bit_size;
@@ -136,7 +138,8 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 		info->flex_array = sym;
 	}
 
-	align_bit_mask = bytes_to_bits(sym->ctype.alignment) - 1;
+	alignment = info->packed ? 1 : sym->ctype.alignment;
+	align_bit_mask = bytes_to_bits(alignment) - 1;
 
 	/*
 	 * Bitfields have some very special rules..
@@ -147,7 +150,7 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 		// Zero-width fields just fill up the unit.
 		int width = base_size ? : (bit_offset ? room : 0);
 
-		if (width > room) {
+		if (width > room && !info->packed) {
 			bit_size = (bit_size + align_bit_mask) & ~align_bit_mask;
 			bit_offset = 0;
 		}
@@ -157,6 +160,8 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 		info->bit_size = bit_size + width;
 		// warning (sym->pos, "bitfield: offset=%d:%d  size=:%d", sym->offset, sym->bit_offset, width);
 
+		if (info->packed && sym->type == SYM_NODE)
+			sym->packed = 1;
 		return;
 	}
 
@@ -173,6 +178,7 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 static struct symbol * examine_struct_union_type(struct symbol *sym, int advance)
 {
 	struct struct_union_info info = {
+		.packed = sym->packed,
 		.max_align = 1,
 		.bit_size = 0,
 		.align_size = 1
@@ -191,7 +197,7 @@ static struct symbol * examine_struct_union_type(struct symbol *sym, int advance
 			sparse_error(info.flex_array->pos, "flexible array member '%s' is not last", show_ident(info.flex_array->ident));
 		examine_symbol_type(member);
 
-		if (member->ctype.alignment > info.max_align) {
+		if (member->ctype.alignment > info.max_align && !sym->packed) {
 			// Unnamed bitfields do not affect alignment.
 			if (member->ident || !is_bitfield_type(member))
 				info.max_align = member->ctype.alignment;
diff --git a/symbol.h b/symbol.h
index 866d57522f49..15b21452c934 100644
--- a/symbol.h
+++ b/symbol.h
@@ -112,6 +112,7 @@ struct decl_state {
 	unsigned char prefer_abstract;
 	unsigned char autotype;
 	unsigned char forced;
+	unsigned char packed;
 };
 
 struct pseudo;
diff --git a/validation/packed-bitfield0.c b/validation/packed-bitfield0.c
index f84e7b904a82..2e20916176f1 100644
--- a/validation/packed-bitfield0.c
+++ b/validation/packed-bitfield0.c
@@ -55,5 +55,4 @@ int main(void)
 
 /*
  * check-name: packed-bitfield0
- * check-known-to-fail
  */
diff --git a/validation/packed-bitfield1.c b/validation/packed-bitfield1.c
index 208a3dc5127c..b7b575ce6922 100644
--- a/validation/packed-bitfield1.c
+++ b/validation/packed-bitfield1.c
@@ -24,5 +24,4 @@ static int foo(struct s *ptr)
 
 /*
  * check-name: packed-bitfield1
- * check-known-to-fail
  */
diff --git a/validation/packed-bitfield2.c b/validation/packed-bitfield2.c
index 4587ebec5c90..244204c2dd35 100644
--- a/validation/packed-bitfield2.c
+++ b/validation/packed-bitfield2.c
@@ -12,5 +12,4 @@ _Static_assert(sizeof(struct bf2) == 8);
 
 /*
  * check-name: packed-bitfield2
- * check-known-to-fail
  */
diff --git a/validation/packed-bitfield5.c b/validation/packed-bitfield5.c
index 8f44d4c2c277..87dbf9c221a1 100644
--- a/validation/packed-bitfield5.c
+++ b/validation/packed-bitfield5.c
@@ -17,5 +17,4 @@ static int ld(struct s *s)
 /*
  * check-name: packed-bitfield5
  * check-description: is check_access() OK with 'overlapping' packed bitfields?
- * check-known-to-fail
  */
diff --git a/validation/packed-deref0.c b/validation/packed-deref0.c
index 865ad68a4f37..d48ad1ac7505 100644
--- a/validation/packed-deref0.c
+++ b/validation/packed-deref0.c
@@ -20,5 +20,4 @@ static void bar(obj_t o)
 
 /*
  * check-name: packed-deref0
- * check-known-to-fail
  */
diff --git a/validation/packed-struct.c b/validation/packed-struct.c
index e21d11538639..dad22791060b 100644
--- a/validation/packed-struct.c
+++ b/validation/packed-struct.c
@@ -29,5 +29,4 @@ _Static_assert(   sizeof(struct c) == 6, "size struct");
 
 /*
  * check-name: packed-struct
- * check-known-to-fail
  */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 00/16] support __packed struct
  2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
                   ` (15 preceding siblings ...)
  2020-12-31 10:10 ` [PATCH v3 16/16] packed: add support for __packed struct Luc Van Oostenryck
@ 2020-12-31 15:30 ` Ramsay Jones
  16 siblings, 0 replies; 18+ messages in thread
From: Ramsay Jones @ 2020-12-31 15:30 UTC (permalink / raw)
  To: Luc Van Oostenryck, linux-sparse



On 31/12/2020 10:10, 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-v3
> 
> 
> Changes since v2 (all thanks to Ramsay Jones):
> * correct several typos
> * add an explanation for one of the test
> * avoid sizeof() in tests where the size is constrained to some fixed value
> * remove a redundency in a test (struct sb identical to struct sa)
> * reformulate the commit message for apply_ctype()'s arguments reversal
> * tweak the not-so-1-to-1 code movement
> * fix the truncated commit message of the last patch

This all LGTM. Thanks!

ATB,
Ramsay Jones

> 
> 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
> 
>  Documentation/TODO.md             |  3 --
>  linearize.c                       | 13 +++++-
>  parse.c                           | 78 ++++++++++++++++---------------
>  symbol.c                          | 12 +++--
>  symbol.h                          |  2 +
>  validation/enum-type-dubious.c    | 18 +++++++
>  validation/enum-type-exotic.c     | 28 +++++++++++
>  validation/packed-bitfield0.c     | 58 +++++++++++++++++++++++
>  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  | 15 ++++++
>  20 files changed, 447 insertions(+), 45 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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-12-31 15:31 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-31 10:10 [PATCH v3 00/16] support __packed struct Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 01/16] add testcases for dubious enum values Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 02/16] add testcases for exotic " Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 03/16] add testcases for enum attributes Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 04/16] add testcases for type attributes Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 05/16] add testcases for packed structures Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 06/16] add testcases for packed bitfields Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 07/16] apply_ctype: use self-explanatory argument name Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 08/16] apply_ctype: reverse the order of arguments Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 09/16] apply_ctype: move up its declaration Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 10/16] struct-attr: prepare to handle attributes at the end of struct definitions (1) Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 11/16] struct-attr: prepare to handle attributes at the end of struct definitions (2) Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 12/16] struct-attr: prepare to handle attributes at the end of struct definitions (3) Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 13/16] struct-attr: fix type attribute like 'struct __attr { ... }' Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 14/16] struct-attr: fix: do not ignore struct/union/enum type attributes Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 15/16] packed: no out-of-bound access of packed bitfields Luc Van Oostenryck
2020-12-31 10:10 ` [PATCH v3 16/16] packed: add support for __packed struct Luc Van Oostenryck
2020-12-31 15:30 ` [PATCH v3 00/16] support " Ramsay Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).