Linux-Sparse Archive mirror
 help / color / mirror / Atom feed
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: linux-sparse@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Subject: [PATCH v2] handle qualified anonymous structures
Date: Fri, 22 Jan 2021 22:23:23 +0100	[thread overview]
Message-ID: <20210122212323.80203-1-luc.vanoostenryck@gmail.com> (raw)

The kernel is trying to use a unnamed struct to mark a group of
fields as being 'const' and get a warning if one of its member is
assigned. GCC gives indeed a warning but Sparse and clang don't.

So, the type qualifiers of the anonymous struct need to be propagate
to the members.

An easy, one-liner, solution is to handle this inside find_identifier(),
where access to the members are recursively searched inside sub-structures.
It's very easy but it feels wrong to do semantics inside this function.
Worse, it's only working for members that are effectively accessed,
doing a type evaluation on the anonymous struct (or its parent)
would, by itself, not handle this.

So, the solution chosen here is to handle this during type examination,
more precisely, inside examine_struct_union_type(), where things are
a bit more complicated (it can't be done when examining the members
themselves because only the parent SYM_STRUCT is accessible and the
qualifiers are in the SYM_NODE, so it needs to be done when examining
the anonymous struct itself) but can be done for all members.

Note: It would seem logical to also handle at least all qualifier-like
      attributes but GCC seems to only bother with the true qualifiers
      and ignore things like attributes and alignment specifiers.

Link: lore.kernel.org/r/CAHk-=wj4Kviw8q2Sx9vrrvyn3uWK-zNi4uGRx=5bzde0Cio8uQ@mail.gmail.com
Link: lore.kernel.org/r/CAHk-=wjdJmL22+zk3_rWAfEJJCf=oDxiJ530qk-WNk_Ji0qhxw@mail.gmail.com
Thanks-to: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---

Change since v1:
* factor out the modifiers from the 'sub' loop
* remove the sym-vs-parent thing thanks to the previous point
* check that sub-members are always a SYM_NODE (via an assert, sorry)

 symbol.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/symbol.c b/symbol.c
index aa02c8c5ad80..91352a3a447b 100644
--- a/symbol.c
+++ b/symbol.c
@@ -25,6 +25,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
+#include <assert.h>
 
 #include "lib.h"
 #include "allocate.h"
@@ -175,6 +176,31 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 	// warning (sym->pos, "regular: offset=%d", sym->offset);
 }
 
+///
+// propagate properties of anonymous structs or unions into their members.
+//
+// :note: GCC seems to only propagate the qualifiers.
+// :note: clang doesn't propagate anything at all.
+static void examine_anonymous_member(struct symbol *sym)
+{
+	unsigned long mod = sym->ctype.modifiers & MOD_QUALIFIER;
+	struct symbol *sub;
+
+	if (sym->type == SYM_NODE)
+		sym = sym->ctype.base_type;
+	if (sym->type != SYM_STRUCT && sym->type != SYM_UNION)
+		return;
+
+	FOR_EACH_PTR(sym->symbol_list, sub) {
+		assert(sub->type == SYM_NODE);
+		sub->ctype.modifiers |= mod;
+
+		// if nested, propagate all the way down
+		if (!sub->ident)
+			examine_anonymous_member(sub);
+	} END_FOR_EACH_PTR(sub);
+}
+
 static struct symbol * examine_struct_union_type(struct symbol *sym, int advance)
 {
 	struct struct_union_info info = {
@@ -196,6 +222,8 @@ static struct symbol * examine_struct_union_type(struct symbol *sym, int advance
 		if (info.flex_array)
 			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->ident)
+			examine_anonymous_member(member);
 
 		if (member->ctype.alignment > info.max_align && !sym->packed) {
 			// Unnamed bitfields do not affect alignment.
-- 
2.30.0


                 reply	other threads:[~2021-01-22 21:24 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210122212323.80203-1-luc.vanoostenryck@gmail.com \
    --to=luc.vanoostenryck@gmail.com \
    --cc=linux-sparse@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).