Address of dllimport symbol is not constexpr

[Quuxplusone]

Bugzilla Link	PR51558
Status	NEW
Importance	P enhancement
Reported by	Sami Kyostila (skyostil@chromium.org)
Reported on	2021-08-20 06:43:32 -0700
Last modified on	2021-09-20 10:39:28 -0700
Version	trunk
Hardware	PC Windows NT
CC	blitzrakete@gmail.com, dgregor@apple.com, erik.pilkington@gmail.com, hans@chromium.org, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk, rnk@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

The following C++ program fails to compile on the x64_64-pc-windows-msvc target:

  extern __declspec(dllimport) int x;
  constexpr int* y = &x;

Error message:

  test.cc(7,16): error: constexpr variable 'y' must be initialized by a constant expression

This is on Clang version 14.0.0 (https://github.com/llvm/llvm-project/
ee65938357d5fffe9e586fa155b37268b5a358ac), target: x86_64-pc-windows-msvc. I
ran into this while trying to compile Perfetto (https://perfetto.dev) as a part
of Chromium.

The same program builds correctly on MSVC (tested on 19.29.30037 for x64).
Interestingly, MSVC had a regression related to this in 2018[1] which has since
been fixed.

[1] https://developercommunity.visualstudio.com/t/c-regression-stdmax-of-two-
constexpr-does-not-eval/312456

[Quuxplusone]

Thanks for the interesting bug report!

From the linked MSVC discussion:

"I conferred with other members of the team, and due to the way DLLs work,
there's debate on whether constexpr + __declspec(dllimport) symbols should even
be permitted (and whether it's technically possible for the two features to
work together)."

This was my initial reaction too, and matches what Clang currently believes:
https://github.com/llvm/llvm-project/blob/llvmorg-12.0.1/clang/test/SemaCXX/dllimport-constexpr.cpp#L40

I think this is the bit of the standard which says that pointers can be
constant expressions: https://eel.is/c++draft/expr.const#11.2

It's a bit counter-intuitive because the pointer value will not be known at
compile time. At least not in terms of an actual address, but if one thinks of
it as a symbolic constant, maybe it makes more sense. And then the fact that
the address isn't available until *load* time doesn't really change anything I
think.

Anyway, the code without dllimport on Linux has the same issue really:

  extern int x;
  constexpr int* y = &x;

x might be accessed through the GOT from a shared library. But Clang and GCC
accepts that just fine. So it would make sense for the dllimport case to be the
same.

[Quuxplusone]

This was my initial reaction too, and matches what Clang currently believes: https://github.com/llvm/llvm-project/blob/llvmorg-12.0.1/clang/test/SemaCXX/dllimport-constexpr.cpp#L40

MSVC compiles that file without any errors. I tried VS 2019 (19.28.29336), VS 2017 (19.12.25834) and VS 2015 (19.00.24215.1).

[Quuxplusone]

> Anyway, the code without dllimport on Linux has the same issue really:
>
>  extern int x;
>  constexpr int* y = &x;
>
> x might be accessed through the GOT from a shared library. But Clang and GCC
accepts that just fine. So it would make sense for the dllimport case to be the
same.

Ah, no it's not the same. In this example:

  extern int x;
  constexpr int *px = &x;
  const int *ptrs[] = { px };

on Linux we will use a dynamic relocation to initialize ptrs at load time.

But on Windows, IIUC, there is no such relocation we can use when x is
dllimported, instead MSVC will do dynamic initialization of ptrs, calling the
__imp_?x thunk. Clang on the other hand will reject the code.

[Quuxplusone]

(In reply to Hans Wennborg from comment #3)
> In this example:
>
>   extern int x;
>   constexpr int *px = &x;
>   const int *ptrs[] = { px };
>
> on Linux we will use a dynamic relocation to initialize ptrs at load time.
>
> But on Windows, IIUC, there is no such relocation we can use when x is
> dllimported, instead MSVC will do dynamic initialization of ptrs, calling
> the __imp_?x thunk. Clang on the other hand will reject the code.

I was wrong. MSVC does do dynamic initialization of 'ptrs', but it still tries
to link directly against 'x' for the initialization of 'px'.

Notes about changes in this area:

https://github.com/llvm/llvm-project/commit/0c43d8077e60
"AST: Initialization with dllimport functions in C"
I think that's the first change on this subject.

http://reviews.llvm.org/D4299
"Don't allow dllimport variables in constant initializers"
Because they're not constant, and clang would generate wrong code for them.

https://reviews.llvm.org/D43320
"Allow dllimport non-type template arguments in C++17"
That was for Bug 35772, which has this comment from rnk:

---
The intention is to make dllimport addresses non-constexpr to reject code like
this:

int __declspec(dllimport) imported;
constexpr int *pi = &imported;
int useit() { return pi; }

MSVC accepts this, which is a shame, but it will fail at link time unless
somebody provides a non-dllimport definition of 'imported'.

Presumably C++17 requires that non-type template parameters obey the constexpr
rules now, so that's why we're rejecting this stuff in 17 and not 14.

Obviously, we shouldn't reject template instantiation with dllimport non-type
template arguments.
---

For that example, the situation still looks the same as in the comment: MSVC
will compile but most likely fail to link that example.

Here's the example I've been looking at:

---
extern int __declspec(dllimport) imported;
constexpr int *pi = &imported;
const int *api[] = { pi };
---

Like above, MSVC will try to link against 'imported' and fail. If we drop the
constexpr, it will correctly dynamically initialize 'pi' (and then 'api') and
things will work.

So, if we wanted to support this, we could make Clang dynamically initialize
'pi' also when it's a constexpr. It would be weird though, because one of the
motivations for constexpr was specifically to guarantee static initialization,
so that would go against developer expectations.

Since MSVC doesn't support this very well, it seems that Clang's current
behavior of rejecting it is the right one?

[Quuxplusone]

First of all, I think clang's diagnostic can be improved. It should say more
than "not a constant expression", it should say something like "the address of
a dllimported global is not constexpr" or something like that. I believe the
constant evaluator even has a mechanism for attaching a note to point at the
non-constant part of the evaluation.

For the behavior, I think it all comes down to how much you care about dynamic
initialization. Maybe my perspective is warped from working on compilers, but I
always felt like that was a *huge* part of the value of constexpr: if I mark a
global constexpr, I can rely on it being initialized before other initializers
run. It's possible that users don't care as much about that as they care about
doing the rest of the computation at compile time.

One way we could try to thread the needle here is to dynamically initialize
constexpr globals before other globals. We could use the priority field of
@llvm.global_ctors to initialize constexpr globals before regular globals, and
most casual users wouldn't be able to tell the difference. I could also imagine
emitting a warning about this, and letting the user decide if they care.