This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Support pointer arithmetic in binary operators
ClosedPublic

Authored by tbaeder on Oct 13 2022, 1:29 AM.

Diff Detail

Event Timeline

tbaeder created this revision.Oct 13 2022, 1:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 13 2022, 1:29 AM
tbaeder requested review of this revision.Oct 13 2022, 1:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 13 2022, 1:29 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
shafik added inline comments.Oct 13 2022, 2:46 PM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
241

I am not sure if this is the right place to handle this but there are a bunch of other cases.

  • nullptr can have 0 added or subtracted
  • You can only do addition/subtraction from a pointer if the result in within bounds or one after the end
  • You can subtract two pointers if they point to the same object.

godbolt: https://godbolt.org/z/5YTY93z8M

tbaeder added inline comments.Oct 14 2022, 1:01 AM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
241

I will probably move this special case out of the function and into its own.
Thanks for the tests, I think this should all already work except for adding two pointers. I'll update the diff once I checked that out.

tbaeder updated this revision to Diff 468058.Oct 15 2022, 11:33 PM
tbaeder updated this revision to Diff 468060.Oct 16 2022, 2:16 AM
tbaeder added inline comments.Oct 16 2022, 2:17 AM
clang/lib/AST/Interp/Interp.h
970 ↗(On Diff #468060)

This is also not being diagnosed (only rejected) by the current interpreter. But would be nice to have an error message.

tbaeder retitled this revision from [clang][Interp] Support pointer arithmethic in binary operators to [clang][Interp] Support pointer arithmetic in binary operators.Oct 16 2022, 9:46 AM
shafik added inline comments.Oct 24 2022, 9:39 PM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
241

You will also want to cover this case as well: https://eel.is/c++draft/expr.add#6

261

or neither left or right operand is a pointer type

273

If it is two pointers then it must be a subtraction.

302
306
310
tbaeder updated this revision to Diff 470400.Oct 25 2022, 1:14 AM
tbaeder marked 5 inline comments as done.
tbaeder marked an inline comment as done.Oct 27 2022, 11:47 PM
tbaeder added inline comments.
clang/lib/AST/Interp/ByteCodeExprGen.cpp
241

FWIW I've looked at the link but can hardly wrap my head around what the text is talking about, let alone make some contant expression out of it that doesn't run into some sort of other error :(

shafik added inline comments.Oct 28 2022, 11:07 AM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
241

nvm, I just realized it requires a reinterpret_cast

shafik added inline comments.Oct 28 2022, 11:21 AM
clang/test/AST/Interp/arrays.cpp
69

We should also have a test for addition out of bounds but subsequent addition bringing it back in bounds. Still UB but we should verify it: https://godbolt.org/z/c1MWjGdfT

Apparently gcc does not catch this one.

shafik added inline comments.Oct 28 2022, 11:23 AM
clang/lib/AST/Interp/Interp.h
970 ↗(On Diff #468060)

The TODO is still present, will you tackle this as part of this PR or a follow-up?

tbaeder updated this revision to Diff 471711.Oct 28 2022, 9:26 PM
tbaeder marked 2 inline comments as done.
tbaeder added inline comments.Oct 28 2022, 9:28 PM
clang/lib/AST/Interp/Interp.h
970 ↗(On Diff #468060)

A follow-up would be better I think.

shafik accepted this revision.Oct 30 2022, 1:14 PM

LGTM

This revision is now accepted and ready to land.Oct 30 2022, 1:14 PM
This revision was landed with ongoing or failed builds.Nov 6 2022, 10:49 PM
This revision was automatically updated to reflect the committed changes.