Linux-Sparse Archive mirror
 help / color / mirror / Atom feed

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc: Sparse Mailing-list <linux-sparse@vger.kernel.org>
Subject: 'const' unnamed structures
Date: Wed, 20 Jan 2021 11:21:46 -0800	[thread overview]
Message-ID: <CAHk-=wj4Kviw8q2Sx9vrrvyn3uWK-zNi4uGRx=5bzde0Cio8uQ@mail.gmail.com> (raw)

So the kernel was trying to use a unnamed struct to mark a group of
fields as being 'const', and while gcc is perfectly happy with that,
it turns out clang is not.

And while testing, I notice that sparse is also not happy with it.

Stupid test-case:

    struct dummy {
        const struct {
                int a, b;
        };
        int c;
    };

    void test(struct dummy *a)
    {
        a->a = a->c;
    }

and gcc will report

    t.c: In function ‘test’:
    t.c:10:7: error: assignment of member ‘a’ in read-only object
       10 |  a->a = a->c;
          |       ^

which is the helpful message we're looking for in the kernel, but
clang and sparse both silently accept this. Presumably for very
similar reasons.

(And when I talk about 'const', obviously all the same issues hold wrt
'volatile' too)

Basically, I see two possibilities

 (a) since the const is there in the unnamed sub-struct, we can "fix"
it at evaluate lo time, in find_identifier().

     We'd have to add a "unsigned int qual" as an argument - initially
zero - to "find_identifier()", and then in the recursive unnamed
union/struct case we'd or in the qualifiers for this one.

     And then we'd modify the symbol result as per the qualifier when
we return it.

Honestly, (a) strikes me as ugly and wrong, but it might be simpler
than what I think might be the right model:

 (b) examine_struct_union_type() knows when it is traversing an
unnamed union,  and when it does that

                examine_symbol_type(member);

     it would then add the modifiers after-the-fact.

Obviously, the third possibility is to say "ok, clang also gets this
wrong, the clang people are trying to argue that the standard is not
clear about it, and sparse might as well ignore this until it's a
bigger problem".

           Linus

next             reply	other threads:[~2021-01-20 19:23 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-20 19:21 Linus Torvalds [this message]
2021-01-20 22:55 ` 'const' unnamed structures Luc Van Oostenryck
2021-01-22 16:26 ` [PATCH] handle qualified anonymous structures Luc Van Oostenryck
2021-01-22 17:35   ` Linus Torvalds
2021-01-22 18:01     ` Linus Torvalds
2021-01-22 20:11       ` Luc Van Oostenryck
2021-01-22 19:38     ` Luc Van Oostenryck

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='CAHk-=wj4Kviw8q2Sx9vrrvyn3uWK-zNi4uGRx=5bzde0Cio8uQ@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=luc.vanoostenryck@gmail.com \
    /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

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).