llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.05k stars 11.98k forks source link

[ExprConst] Field access resulting in null pointer is diagnosed at compile time #113821

Open tbaederr opened 2 weeks ago

tbaederr commented 2 weeks ago

Sorry for the clunky title.

Consider this code:

  constexpr struct C {int a[2];} *c = nullptr;
  static_assert(&c->a[1] - &c->a[0] == 1);

The output is:

./array.cpp:141:17: error: static assertion expression is not an integral constant expression
  141 |   static_assert(&c->a[1] - &c->a[0] == 1);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~
./array.cpp:141:21: note: cannot access field of null pointer
  141 |   static_assert(&c->a[1] - &c->a[0] == 1);
      |                     ^

We diagnose this here: https://github.com/llvm/llvm-project/blob/7fe149cdf09d04fb8390b97c91bd9214c968cd3e/clang/lib/AST/ExprConstant.cpp#L1715-L1720

Because IsNullPtr is true here: https://github.com/llvm/llvm-project/blob/7fe149cdf09d04fb8390b97c91bd9214c968cd3e/clang/lib/AST/ExprConstant.cpp#L1702-L1712

However, if the field we're talking about coincidentally doesn't start at offset 0 (and thus c->a isn't 0), we don't diagnose at all and this example just works:

  constexpr struct C {char pad; int a[2];} *c = nullptr;
  static_assert(&c->a[1] - &c->a[0] == 1);

https://godbolt.org/z/vzTr6sEMo

As you can see in the godbolt link, GCC rejects this as well.

I'm wondering that is right here. Should the code work in both cases or in neither case?

CC @zygoloid @AaronBallman

llvmbot commented 2 weeks ago

@llvm/issue-subscribers-clang-frontend

Author: Timm Baeder (tbaederr)

Sorry for the clunky title. Consider this code: ```c++ constexpr struct C {int a[2];} *c = nullptr; static_assert(&c->a[1] - &c->a[0] == 1); ``` The output is: ```console ./array.cpp:141:17: error: static assertion expression is not an integral constant expression 141 | static_assert(&c->a[1] - &c->a[0] == 1); | ^~~~~~~~~~~~~~~~~~~~~~~~ ./array.cpp:141:21: note: cannot access field of null pointer 141 | static_assert(&c->a[1] - &c->a[0] == 1); | ^ ``` We diagnose this here: https://github.com/llvm/llvm-project/blob/7fe149cdf09d04fb8390b97c91bd9214c968cd3e/clang/lib/AST/ExprConstant.cpp#L1715-L1720 Because `IsNullPtr` is true here: https://github.com/llvm/llvm-project/blob/7fe149cdf09d04fb8390b97c91bd9214c968cd3e/clang/lib/AST/ExprConstant.cpp#L1702-L1712 However, if the field we're talking about coincidentally doesn't start at offset 0 (and thus `c->a` isn't `0`), we don't diagnose at all and this example just works: ``` constexpr struct C {char pad; int a[2];} *c = nullptr; static_assert(&c->a[1] - &c->a[0] == 1); ``` https://godbolt.org/z/vzTr6sEMo As you can see in the godbolt link, GCC rejects this as well. I'm wondering that is right here. Should the code work in both cases or in neither case? CC @zygoloid @AaronBallman
zygoloid commented 2 weeks ago

It should work in neither case. Member access is only defined when accessing a subobject of an object, and nullptr doesn't point to an object. Looks like we're missing a check in the handling of ->.

tbaederr commented 2 weeks ago

Simply changing the check from IsNullPtr to getLValueBase().isNull() seems to do the trick.