Skip to content

clang permits write to const anonymous struct members #48099

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nickdesaulniers opened this issue Jan 14, 2021 · 10 comments
Open

clang permits write to const anonymous struct members #48099

nickdesaulniers opened this issue Jan 14, 2021 · 10 comments
Labels
bugzilla Issues migrated from bugzilla c clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party

Comments

@nickdesaulniers
Copy link
Member

nickdesaulniers commented Jan 14, 2021

Bugzilla Link 48755
Version trunk
OS All
Blocks #4440
CC @dwblaikie,@DougGregor,@zygoloid

Extended Description

Via this LKML thread: https://lore.kernel.org/lkml/[email protected]/T/#m1328d75e335d7e58bdd670ca30fea062b738f408

(Reported-by: Will Deacon [email protected])

struct foo {
    const struct {
        int bar;
    };
} my_foo;

struct foo2 {
    const struct {
        int bar;
    } named;
} my_foo2;

void baz (void) {
    my_foo.bar = 42; // GCC errors, clang does not
    my_foo2.named.bar = 42; // GCC and Clang error
}

Marking bar as const qualified is a portable workaround, for now.

https://godbolt.org/z/h7qPxd

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Jan 15, 2021

C11 6.7.2.1/13: An unnamed member whose type specifier is a structure specifier with no tag is called an anonymous structure; an unnamed member whose type specifier is a union specifier with no tag is called an anonymous union. The members of an anonymous structure or union are considered to be members of the containing structure or union.

C11 6.5.2.3/3: A postfix expression followed by the . operator and an identifier designates a member of a structure or union object. [...] If the first expression has qualified type, the result has the so-qualified version of the type of the designated member.

C11 6.3.2.1/1: A modifiable lvalue is an lvalue that does not have array type, does not have an incomplete type, does not have a const-qualified type, and if it is a structure or union, does not have any member (including, recursively, any member or element of all contained aggregates or unions) with a const- qualified type.

C11 6.5.16/2: An assignment operator shall have a modifiable lvalue as its left operand.

So, it appears that Clang is correct. my_foo has a member 'bar' of type 'int'. The type of 'my_foo.bar' is 'int'. 'my_foo.bar' is a modifiable lvalue. And so assigning to it is valid.

Qualifiers on an anonymous struct or union appear to have no effect in C11 (and are invalid in C++, which Clang rejects in pedantic mode and GCC apparently does not: https://godbolt.org/z/dqh6P6).

FWIW, GCC accepts 'my_foo.bar = 42;' in C++ mode: https://godbolt.org/z/ooPv6n

Keeping this bug open: I think this code deserves at least a warning by default (for the ignored qualifiers on the anonymous struct member). And maybe someone should ask WG14 if this is what they wanted.

@nickdesaulniers
Copy link
Member Author

Keeping this bug open: I think this code deserves at least a warning by default

diag::ext_anonymous_struct_union_qualified seems to be used for this, but looks like it's guarded by if (getLangOpts().CPlusPlus).

So, it appears that Clang is correct.

Should I file a GCC bug then?

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Jan 15, 2021

Keeping this bug open: I think this code deserves at least a warning by default

diag::ext_anonymous_struct_union_qualified seems to be used for this, but
looks like it's guarded by if (getLangOpts().CPlusPlus).

Yeah, that's the extension warning for the -pedantic diagnostic. We'll want a separate diagnostic for C because the code is valid (though surprising).

So, it appears that Clang is correct.

Should I file a GCC bug then?

Sounds reasonable. Maybe they can tell us that GNU C intentionally diverges from ISO C here? :)

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
@AaronBallman
Copy link
Collaborator

In the meantime, I'm going to ask on the WG14 mailing lists whether this is intentional or not.

@EugeneZelenko EugeneZelenko added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label May 5, 2022
@llvmbot
Copy link
Member

llvmbot commented May 5, 2022

@llvm/issue-subscribers-clang-frontend

@AaronBallman AaronBallman added the confirmed Verified by a second party label May 5, 2022
@AaronBallman
Copy link
Collaborator

The initial sentiment coming back on the reflectors is that assigning to my_foo.bar should be a constraint violation because it should have type const int, and the standard should either be clarified or fixed in that direction. If there's strong rationale from anyone contrary to that, I'll report back. However, I feel comfortable confirming this as a bug at this point.

@AaronBallman
Copy link
Collaborator

@AaronBallman
Copy link
Collaborator

Okay, so I was comfortable confirming this as a bug, but I'm less comfortable now that I see GCC is the only C compiler I can find that considers the anonymous object as part of the access path: https://godbolt.org/z/hTqY8zMb5

I'm pushing back on the WG14 lists to see if this is a good opportunity to clarify the other direction and be compatible with C++ in the process.

@nickdesaulniers
Copy link
Member Author

(Forgot I had https://reviews.llvm.org/D95408 for this; will abandon in favor of https://reviews.llvm.org/D125167)

@nickdesaulniers
Copy link
Member Author

@AaronBallman did https://reviews.llvm.org/D125167 slip through the cracks?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla c clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party
Projects
None yet
Development

No branches or pull requests

4 participants