Skip to content
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

<compare>: std::_Literal_zero has an impact on real code. #4359

Open
fsb4000 opened this issue Jan 31, 2024 · 13 comments
Open

<compare>: std::_Literal_zero has an impact on real code. #4359

fsb4000 opened this issue Jan 31, 2024 · 13 comments
Labels
external This issue is unrelated to the STL

Comments

@fsb4000
Copy link
Contributor

fsb4000 commented Jan 31, 2024

Describe the bug

Our type std::_Literal_zero has an impact on real code.

From the comment: #4332 (comment)

Command-line test case

#include <compare>

template<typename T, typename U>
constexpr bool not_equal(T lhs, U rhs) 
requires(requires(T lhs, U rhs) { lhs != rhs; })
{
        return lhs != rhs;
}

template<typename T, typename U>
constexpr bool not_equal(T lhs, U rhs) 
requires(!requires(T lhs, U rhs) { lhs != rhs; })
{
        return false;
}

int main() {
    constexpr int int_zero = 0;
    constexpr unsigned int unsigned_zero = 0;
    constexpr auto ordering = 1 <=> 1;
    //constexpr bool b = not_equal(ordering, unsigned_zero);
    constexpr bool b = not_equal(ordering, int_zero);
    return b == false;
}


C:\Temp>cl /EHsc /W4 /WX /std:c++latest .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.39.33321 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

/std:c++latest is provided as a preview of language features from the latest C++
working draft, and we're eager to hear about bugs and suggestions for improvements.
However, note that these features are provided as-is without support, and subject
to changes or removal as the working draft evolves. See
https://go.microsoft.com/fwlink/?linkid=2045807 for details.

repro.cpp
.\repro.cpp(7): error C7595: 'std::_Literal_zero::_Literal_zero': call to immediate function is not a constant expression
.\repro.cpp(7): note: failure was caused by a read of a variable outside its lifetime
.\repro.cpp(7): note: see usage of 'rhs'
.\repro.cpp(7): note: the template instantiation context (the oldest one first) is
.\repro.cpp(22): note: see reference to function template instantiation 'bool not_equal<std::strong_ordering,int>(T,U)' being compiled
        with
        [
            T=std::strong_ordering,
            U=int
        ]

Expected behavior

Should compile

STL version

202e382

Additional context

Other compiler libraries compile the code: https://godbolt.org/z/1vbYbq5b7

We added std::_Literal_zero because of this issue: #3581

Previously using _Literal_zero = decltype(nullptr); warned this code:

#include <compare>

auto b = 1 <=> 2 < 0;

int main() {}
clang-cl /std:c++20 -Wzero-as-null-pointer-constant main.cpp
main.cpp(3,20): warning: zero as null pointer constant [-Wzero-as-null-pointer-constant]
auto b = 1 <=> 2 < 0;
                   ^
                   nullptr
1 warning generated.
@frederick-vs-ja
Copy link
Contributor

It's unfortunate that [cmp.categories.pre]/3 doesn't seem to limit the evilness of the unspecified parameter type...

@StephanTLavavej
Copy link
Member

It sure looks like this is an MSVC compiler consteval bug to me, given that Clang accepts MSVC's STL here:

C:\Temp>type meow.cpp
#include <compare>

template <typename T, typename U>
constexpr bool not_equal(T lhs, U rhs)
    requires (requires(T lhs, U rhs) { lhs != rhs; })
{
    return lhs != rhs;
}

int main() {
    constexpr int int_zero  = 0;
    constexpr auto ordering = 1 <=> 1;
    constexpr bool b        = not_equal(ordering, int_zero);
    return b == false;
}
C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /c meow.cpp
meow.cpp
meow.cpp(7): error C7595: 'std::_Literal_zero::_Literal_zero': call to immediate function is not a constant expression
meow.cpp(7): note: failure was caused by a read of a variable outside its lifetime
meow.cpp(7): note: see usage of 'rhs'
meow.cpp(7): note: the template instantiation context (the oldest one first) is
meow.cpp(13): note: see reference to function template instantiation 'bool not_equal<std::strong_ordering,int>(T,U)' being compiled
        with
        [
            T=std::strong_ordering,
            U=int
        ]

C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++latest /c meow.cpp

C:\Temp>

@StephanTLavavej StephanTLavavej added the external This issue is unrelated to the STL label Jan 31, 2024
@frederick-vs-ja
Copy link
Contributor

Oh, I think this is probably because of that MSVC hasn't implemented WG21-P2564R3 (DR against C++20).

Reduced example (Godbolt link):

struct Par {
    consteval Par(int) {}
};

template<class I>
constexpr bool testfun(I n)
{
    (void) Par{n};
    return true;
}

int main()
{
    static_assert(testfun(0));
}

lhs != rhs triggers implicit conversion from int to _Literal_zero, which calls an immediate (consteval) function.

Until P2564R3, the immediate function call is required to produce a constant expression, while reading from a function parameter makes the function call non-constant, which would in turn results in compilation error.
Since P2564R3, the specialization of outer function template not_equal/testfun is automatically made immediate ("immediate-escalation"), so lhs != rhs becomes well-formed.

@horenmar
Copy link
Contributor

horenmar commented Feb 2, 2024

Yeah, I am not happy about not being able to SFINAE on the type, but after MSVC implements P2564 I can solve my issue by constexprifying the relevant parts of Catch2.

@fsb4000
Copy link
Contributor Author

fsb4000 commented Feb 3, 2024

I found a Devcom issue about P2564R3: Devcom-10514467
I added a comment about the current issue.

@fsb4000
Copy link
Contributor Author

fsb4000 commented Feb 13, 2024

@bebuch

Hello @cdacamar , sorry to bother you, but could you please let me know if the implementation of P2564R3 is in your team near plans or if you are currently busy with other work and the implementation is not planned for the near future? You can respond here or in the Devcom-10514467. Thank you in advance!

@cdacamar
Copy link
Contributor

cdacamar commented Feb 13, 2024

I can't provide any specifics as to when this DR will be implemented, but since it was approved in Kona in 2022 we have been tracking it as part of our upcoming C++23 work. I expect us to start work on C++23 stuff very soon.

For STL maintainers with internal query access, the User Story for the feature is VSO-1676035.

@cor3ntin
Copy link

I can't provide any specifics as to when this DR will be implemented, but since it was approved in Kona in 2022 we have been tracking it as part of our upcoming C++23 work. I expect us to start work on C++23 stuff very soon.

That was pretty mind bending to implement, good luck :D

@GkvJwa
Copy link

GkvJwa commented Apr 8, 2024

@bebuch

Hello @cdacamar , sorry to bother you, but could you please let me know if the implementation of P2564R3 is in your team near plans or if you are currently busy with other work and the implementation is not planned for the near future? You can respond here or in the Devcom-10514467. Thank you in advance!

Hello, I have a problem.
When using clang and stl library of msvc on windows, in the debug version, using the default operator<=> will generate, the following code

std::_Literal_zero *__fastcall std::_Literal_zero::_Literal_zero(std::_Literal_zero *this, int a2)
{
  if ( a2 )
    std::_Literal_zero_is_expected();
  return this;
}

the _Literal_zero_is_expected function is used as an exported function.

                cmp     [rsp+38h+var_C], 0
                jz      loc_42EF
                call    ?_Literal_zero_is_expected@std@@YAXXZ ; std::_Literal_zero_is_expected(void)

loc_42EF:                               ; CODE XREF: std::_Literal_zero::_Literal_zero(int)+1C↑j
                mov     rax, [rsp+38h+var_8]
                add     rsp, 38h
                retn
??$?0H$0A@@_Literal_zero@std@@QEAA@H@Z endp

I found the following information
patch_msvc
patch_llvm
From these links, it seems that it is to throw an exception
It is not merged into llvm, but merged into msvc stl
So which lib has the implementation of this function(_Literal_zero_is_expected) on Windows

_STD_BEGIN
void _Literal_zero_is_expected();

struct _Literal_zero {
    template <class _Ty, enable_if_t<is_same_v<_Ty, int>, int> = 0>
    consteval _Literal_zero(_Ty _Zero) noexcept {
        // Can't use _STL_VERIFY because this is a core header
        if (_Zero != 0) {
            _Literal_zero_is_expected();
        }
    }
};

using _Compare_t = signed char;

Does it mean that this code is not 0(_Zero != 0), causing a link error?

lld-link: error: undefined symbol: void __cdecl std::_Literal_zero_is_expected(void)

@fsb4000
Copy link
Contributor Author

fsb4000 commented Apr 8, 2024

Please give me your full example.

causing a link error

A link error should be only for wrong code. That's the idea of the std::_Literal_zero type.

@frederick-vs-ja
Copy link
Contributor

the _Literal_zero_is_expected function is used as an exported function.

The body of a consteval function should not cause ASM generation. This looks like a bug of Clang (something like LLVM-82154?).

@GkvJwa
Copy link

GkvJwa commented Apr 9, 2024

the _Literal_zero_is_expected function is used as an exported function.

The body of a consteval function should not cause ASM generation. This looks like a bug of Clang (something like LLVM-82154?).

I compiled a latest version of llvm, from the main branch, also encountered the same problem


Please give me your full example.

causing a link error

A link error should be only for wrong code. That's the idea of the std::_Literal_zero type.

Have you compiled the latest chromium on windows, after pulling the main branch, you can compile with the following parameters

gn gen out/stl_bug --args="enable_base_tracing=false clang_use_chrome_plugins=false use_custom_libcxx=false"
ninja -C out/stl_bug base

During the compilation process, you may encounter some compilation errors, but this does not affect the reproduction of this problem
When an error is reported, you can decompile and view the following two files(in src/out/stl_bug)
obj/base/base/waitable_event.obj
obj/base/base/scoped_blocking_call_internal.obj

lld-link: error: undefined symbol: void __cdecl std::_Literal_zero_is_expected(void)
>>> referenced by \VC\Tools\MSVC\14.39.33519\include\compare:35
>>>               obj/base/base/waitable_event.obj:(public: __cdecl std::_Literal_zero::_Literal_zero<int, 0>(int))
>>> referenced by obj/base/base/scoped_blocking_call_internal.obj

Then find a function similar to this

.text:0000000000000740 ; char __fastcall base::operator<=>(__int64, __int64)
.text:0000000000000740                 public ??__Mbase@@YA?AUstrong_ordering@std@@VTimeDelta@0@0@Z
.text:0000000000000740 ??__Mbase@@YA?AUstrong_ordering@std@@VTimeDelta@0@0@Z proc near
.text:0000000000000740                                         ; CODE XREF: base::WaitableEvent::TimedWait(base::TimeDelta)+76↑p

.text:000000000000083D                 xor     edx, edx        ; int
.text:000000000000083F                 call    ??$?0H$0A@@_Literal_zero@std@@QEAA@H@Z ; std::_Literal_zero::_Literal_zero(int)
.text:0000000000000844                 mov     al, [rsp+98h+var_21]
.text:0000000000000848                 mov     [rsp+98h+var_4A], al
.text:000000000000084C                 mov     cl, [rsp+98h+var_4A]

.text:0000000000000E40                 mov     [rsp+38h+var_18], rcx
.text:0000000000000E45                 mov     rax, [rsp+38h+var_18]
.text:0000000000000E4A                 mov     [rsp+38h+var_8], rax
.text:0000000000000E4F                 cmp     [rsp+38h+var_C], 0
.text:0000000000000E54                 jz      loc_E5F
.text:0000000000000E5A                 call    ?_Literal_zero_is_expected@std@@YAXXZ ; std::_Literal_zero_is_expected(void)
.text:0000000000000E5F
.text:0000000000000E5F loc_E5F:                                ; CODE XREF: std::_Literal_zero::_Literal_zero(int)+1C↑j
.text:0000000000000E5F                 mov     rax, [rsp+38h+var_8]
.text:0000000000000E64                 add     rsp, 38h

You can see that the function(_Literal_zero_is_expected) is referenced

In addition, when I want to write a demo to reproduce this problem using base::TimeDelta and base::ClampedNumeric, but the generated code does not introduce _Literal_zero_is_expected(using the same version of clang)
demo:

std::strong_ordering __fastcall base::operator<=>(base::TimeDelta a1, base::TimeDelta a2)
{
  const std::strong_ordering *v3; // [rsp+28h] [rbp-70h]
  const std::strong_ordering *v4; // [rsp+30h] [rbp-68h]
  std::strong_ordering v5; // [rsp+77h] [rbp-21h]

  if ( base::internal::operator==<base::internal::ClampedNumeric<__int64>,base::internal::ClampedNumeric<__int64>>(
         a1.delta_,
         a2.delta_) )
  {
    v4 = &std::strong_ordering::equal;
  }
  else
  {
    if ( base::internal::operator<<base::internal::ClampedNumeric<__int64>,base::internal::ClampedNumeric<__int64>>(
           a1.delta_,
           a2.delta_) )
    {
      v3 = &std::strong_ordering::less;
    }
    else
    {
      v3 = &std::strong_ordering::greater;
    }
    v4 = v3;
  }
  v5._Value = v4->_Value;
  if ( std::operator==((const std::strong_ordering)v4->_Value, (std::_Literal_zero)v4->_Value) )
    return std::strong_ordering::equal;
  else
    return v5;
}

chromium:

std::strong_ordering __fastcall base::operator<=>(base::TimeDelta a1, base::TimeDelta a2)
{
  __int64 v2; // rcx
  __int64 v3; // rdx
  const std::strong_ordering *v5; // [rsp+28h] [rbp-70h]
  std::strong_ordering *v6; // [rsp+30h] [rbp-68h]
  std::_Literal_zero v7; // [rsp+4Fh] [rbp-49h] BYREF
  __int64 v8; // [rsp+50h] [rbp-48h]
  __int64 v9; // [rsp+58h] [rbp-40h]
  __int64 v10; // [rsp+60h] [rbp-38h]
  __int64 v11; // [rsp+68h] [rbp-30h]
  std::strong_ordering v12; // [rsp+77h] [rbp-21h]
  __int64 v13; // [rsp+78h] [rbp-20h]
  __int64 value; // [rsp+80h] [rbp-18h]

  value = a1.delta_.value_;
  v13 = a2.delta_.value_;
  v12._Value = -86;
  v11 = a2.delta_.value_;
  v10 = a1.delta_.value_;
  if ( (base::internal::operator==<base::internal::ClampedNumeric<__int64>,base::internal::ClampedNumeric<__int64>>(
          a1.delta_.value_,
          a2.delta_.value_) & 1) != 0 )
  {
    v6 = (std::strong_ordering *)&std::strong_ordering::equal;
  }
  else
  {
    v9 = v13;
    v8 = value;
    if ( (base::internal::operator<<base::internal::ClampedNumeric<__int64>,base::internal::ClampedNumeric<__int64>>(
            value,
            v13) & 1) != 0 )
      v5 = &std::strong_ordering::less;
    else
      v5 = &std::strong_ordering::greater;
    v6 = (std::strong_ordering *)v5;
  }
  v12._Value = v6->_Value;
  std::_Literal_zero::_Literal_zero(&v7, 0);
  LOBYTE(v2) = v12;
  LOBYTE(v3) = v7;
  if ( (std::operator==(v2, v3) & 1) != 0 )
    return std::strong_ordering::equal;
  else
    return v12;
}

@GkvJwa
Copy link

GkvJwa commented Apr 11, 2024

the _Literal_zero_is_expected function is used as an exported function.

The body of a consteval function should not cause ASM generation. This looks like a bug of Clang (something like LLVM-82154?).

I compiled a latest version of llvm, from the main branch, also encountered the same problem

Please give me your full example.

causing a link error

A link error should be only for wrong code. That's the idea of the std::_Literal_zero type.

Have you compiled the latest chromium on windows, after pulling the main branch, you can compile with the following parameters

gn gen out/stl_bug --args="enable_base_tracing=false clang_use_chrome_plugins=false use_custom_libcxx=false"
ninja -C out/stl_bug base

During the compilation process, you may encounter some compilation errors, but this does not affect the reproduction of this problem When an error is reported, you can decompile and view the following two files(in src/out/stl_bug) obj/base/base/waitable_event.obj obj/base/base/scoped_blocking_call_internal.obj

lld-link: error: undefined symbol: void __cdecl std::_Literal_zero_is_expected(void)
>>> referenced by \VC\Tools\MSVC\14.39.33519\include\compare:35
>>>               obj/base/base/waitable_event.obj:(public: __cdecl std::_Literal_zero::_Literal_zero<int, 0>(int))
>>> referenced by obj/base/base/scoped_blocking_call_internal.obj

Then find a function similar to this

.text:0000000000000740 ; char __fastcall base::operator<=>(__int64, __int64)
.text:0000000000000740                 public ??__Mbase@@YA?AUstrong_ordering@std@@VTimeDelta@0@0@Z
.text:0000000000000740 ??__Mbase@@YA?AUstrong_ordering@std@@VTimeDelta@0@0@Z proc near
.text:0000000000000740                                         ; CODE XREF: base::WaitableEvent::TimedWait(base::TimeDelta)+76↑p

.text:000000000000083D                 xor     edx, edx        ; int
.text:000000000000083F                 call    ??$?0H$0A@@_Literal_zero@std@@QEAA@H@Z ; std::_Literal_zero::_Literal_zero(int)
.text:0000000000000844                 mov     al, [rsp+98h+var_21]
.text:0000000000000848                 mov     [rsp+98h+var_4A], al
.text:000000000000084C                 mov     cl, [rsp+98h+var_4A]

.text:0000000000000E40                 mov     [rsp+38h+var_18], rcx
.text:0000000000000E45                 mov     rax, [rsp+38h+var_18]
.text:0000000000000E4A                 mov     [rsp+38h+var_8], rax
.text:0000000000000E4F                 cmp     [rsp+38h+var_C], 0
.text:0000000000000E54                 jz      loc_E5F
.text:0000000000000E5A                 call    ?_Literal_zero_is_expected@std@@YAXXZ ; std::_Literal_zero_is_expected(void)
.text:0000000000000E5F
.text:0000000000000E5F loc_E5F:                                ; CODE XREF: std::_Literal_zero::_Literal_zero(int)+1C↑j
.text:0000000000000E5F                 mov     rax, [rsp+38h+var_8]
.text:0000000000000E64                 add     rsp, 38h

You can see that the function(_Literal_zero_is_expected) is referenced

In addition, when I want to write a demo to reproduce this problem using base::TimeDelta and base::ClampedNumeric, but the generated code does not introduce _Literal_zero_is_expected(using the same version of clang) demo:

std::strong_ordering __fastcall base::operator<=>(base::TimeDelta a1, base::TimeDelta a2)
{
  const std::strong_ordering *v3; // [rsp+28h] [rbp-70h]
  const std::strong_ordering *v4; // [rsp+30h] [rbp-68h]
  std::strong_ordering v5; // [rsp+77h] [rbp-21h]

  if ( base::internal::operator==<base::internal::ClampedNumeric<__int64>,base::internal::ClampedNumeric<__int64>>(
         a1.delta_,
         a2.delta_) )
  {
    v4 = &std::strong_ordering::equal;
  }
  else
  {
    if ( base::internal::operator<<base::internal::ClampedNumeric<__int64>,base::internal::ClampedNumeric<__int64>>(
           a1.delta_,
           a2.delta_) )
    {
      v3 = &std::strong_ordering::less;
    }
    else
    {
      v3 = &std::strong_ordering::greater;
    }
    v4 = v3;
  }
  v5._Value = v4->_Value;
  if ( std::operator==((const std::strong_ordering)v4->_Value, (std::_Literal_zero)v4->_Value) )
    return std::strong_ordering::equal;
  else
    return v5;
}

chromium:

std::strong_ordering __fastcall base::operator<=>(base::TimeDelta a1, base::TimeDelta a2)
{
  __int64 v2; // rcx
  __int64 v3; // rdx
  const std::strong_ordering *v5; // [rsp+28h] [rbp-70h]
  std::strong_ordering *v6; // [rsp+30h] [rbp-68h]
  std::_Literal_zero v7; // [rsp+4Fh] [rbp-49h] BYREF
  __int64 v8; // [rsp+50h] [rbp-48h]
  __int64 v9; // [rsp+58h] [rbp-40h]
  __int64 v10; // [rsp+60h] [rbp-38h]
  __int64 v11; // [rsp+68h] [rbp-30h]
  std::strong_ordering v12; // [rsp+77h] [rbp-21h]
  __int64 v13; // [rsp+78h] [rbp-20h]
  __int64 value; // [rsp+80h] [rbp-18h]

  value = a1.delta_.value_;
  v13 = a2.delta_.value_;
  v12._Value = -86;
  v11 = a2.delta_.value_;
  v10 = a1.delta_.value_;
  if ( (base::internal::operator==<base::internal::ClampedNumeric<__int64>,base::internal::ClampedNumeric<__int64>>(
          a1.delta_.value_,
          a2.delta_.value_) & 1) != 0 )
  {
    v6 = (std::strong_ordering *)&std::strong_ordering::equal;
  }
  else
  {
    v9 = v13;
    v8 = value;
    if ( (base::internal::operator<<base::internal::ClampedNumeric<__int64>,base::internal::ClampedNumeric<__int64>>(
            value,
            v13) & 1) != 0 )
      v5 = &std::strong_ordering::less;
    else
      v5 = &std::strong_ordering::greater;
    v6 = (std::strong_ordering *)v5;
  }
  v12._Value = v6->_Value;
  std::_Literal_zero::_Literal_zero(&v7, 0);
  LOBYTE(v2) = v12;
  LOBYTE(v3) = v7;
  if ( (std::operator==(v2, v3) & 1) != 0 )
    return std::strong_ordering::equal;
  else
    return v12;
}

I think it may be due to a clang option added by chromium

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external This issue is unrelated to the STL
Projects
None yet
Development

No branches or pull requests

7 participants