Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[clang][Interp] Check pointers when accessing base class
Needs RevisionPublic

Authored by tbaeder on Apr 23 2023, 1:19 AM.

Diff Detail

Event Timeline

tbaeder created this revision.Apr 23 2023, 1:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2023, 1:19 AM
tbaeder requested review of this revision.Apr 23 2023, 1:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2023, 1:19 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
This revision is now accepted and ready to land.May 2 2023, 1:44 PM
aaron.ballman requested changes to this revision.May 2 2023, 1:54 PM
aaron.ballman added inline comments.
clang/test/AST/Interp/records.cpp
509–512

I may have jumped the gun on accepting this, actually. Forming the pointer to &b + 1 is fine, but evaluating it by dereferencing it would be UB. e.g., http://eel.is/c++draft/expr.const#13.3

This revision now requires changes to proceed.May 2 2023, 1:54 PM
cor3ntin added inline comments.
clang/test/AST/Interp/records.cpp
509–512

We probably want tests that ensure &b+2 is invalid in all cases

tbaeder added inline comments.May 2 2023, 11:09 PM
clang/test/AST/Interp/records.cpp
509–512

It's being evaluated because the base class is accessed, isn't it? if b was of type A*, we would not emit a diagnostic (see line 506 above).

aaron.ballman added inline comments.
clang/test/AST/Interp/records.cpp
509–512

I'm not seeing the access to the base class -- we're forming a pointer, not dereferencing it.

https://godbolt.org/z/vxThqczeo

I think this is an existing Clang bug. CC @hubert.reinterpretcast @rsmith for additional opinions.

cor3ntin added inline comments.Aug 10 2023, 6:21 AM
clang/test/AST/Interp/records.cpp
509–512

I believe this is fine, or at least i can't find any wording that would say it's not.

In particular:

So i do believe GCC, MSVC and ICC are correct here.