Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Accidental equality of classes templated by pointer to local static constant of templated function #47486

Open Quuxplusone opened 3 years ago

Quuxplusone commented 3 years ago
Bugzilla Link PR48517
Status REOPENED
Importance P enhancement
Reported by Matthieu Monrocq (matthieum.147192@gmail.com)
Reported on 2020-12-15 08:48:15 -0800
Last modified on 2021-01-06 00:32:50 -0800
Version 5.0
Hardware PC Windows NT
CC blitzrakete@gmail.com, dblaikie@gmail.com, erik.pilkington@gmail.com, llvm-bugs@lists.llvm.org, paul_robinson@playstation.sony.com, richard-llvm@metafoo.co.uk
Fixed by commit(s) rG6b760a50f52142e401a6380ff71f933cda22a909
Attachments
Blocks
Blocked by
See also

Based on https://stackoverflow.com/q/65306562/147192.

The behavior of Clang (and GCC) is inconsistent (between compile-time and run-time), however it is unclear to me whether the inconsistency is conforming with the C++17 standard or not.

Furthermore, in -O0 mode, Clang generates an unused symbol (create()::I) which causes the linker to fail, see https://godbolt.org/z/M4T7f3.

The following reduced program is expected to return 0 (invoking clang++ with -std=c++17), it does not (https://godbolt.org/z/6r6vK3):

template <typename, typename> struct is_same { static constexpr bool value = false; };

template struct is_same<T, T> { static constexpr bool value = true; };

template <typename T, typename U> static constexpr bool is_same_v = is_same<T, U>::value;

using uintptr_t = unsigned long long;

template <int const I> struct Parameterized { int const member; };

template auto create() { static constexpr int const I = 2;

return Parameterized<&I>{ &I };

}

int main() { auto one = create(); auto two = create();

if (is_same_v<decltype(one), decltype(two)>) {
    return reinterpret_cast<uintptr_t>(one.member) == reinterpret_cast<uintptr_t>(two.member) ? 1 : 2;
}

return 0;

}

Yet, on all versions of Clang where it compiles (from 5.0.0 onwards), and for all optimization levels (from -O1 to -O3), it returns 2, indicating:

The assembly listing clearly contains 2 different instances of create<T>()::I.

Notes:

Quuxplusone commented 3 years ago

Thanks for the report, fixed in Clang trunk.

Quuxplusone commented 3 years ago
Hello Richard,

The situation _did_ improve your patch (Thanks!), though the problem is not
_completely_ solved.

Selecting "Clang (trunk)" on godbolt (https://godbolt.org/z/9Ybqq5) which is
based on 4c8c6368710 (2020-12-16 15:38:58 -0800) so after 6b760a50f52 (2020-12-
15 13:23:08 -0800) we can see:

- With -O1 to O3: correct output (0).
- With -O0: linker error, as before.

The specific linker error:

/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/11.0.0/../../../../x86_64-linux-gnu/bin/ld:

/tmp/example-a44fce.o:(.debug_info+0xc2): undefined reference to
`create<short>()::I'

/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/11.0.0/../../../../x86_64-linux-gnu/bin/ld:

/tmp/example-a44fce.o:(.debug_info+0xf1): undefined reference to
`create<int>()::I'

Those symbols looks exactly like what was generated in the assembly in Clang
11.0, however now in trunk the symbols generated in the assembly are:

_ZZ6createIsEDavE1I.1:
        .long   2                               # 0x2

_ZZ6createIiEDavE1I.2:
        .long   2                               # 0x2

These extra .1 and .2 are different from before.

It appears that the linker still expects to find _ZZ6createIsEDavE1I (no .1)
and _ZZ6createIiEDavE1I (no .2), for some reason.
Quuxplusone commented 3 years ago

The remaining issue looks like a bug in debug information generation. If you build with -g0, the problem goes away (and, disturbingly, if you build with -g, the debug info symbols get the proper mangling and the actually-referenced-from-the-code symbols get the wrong .1 / .2 manglings). It's not clear to me if the problem goes deeper than debug info generation, though.

Quuxplusone commented 3 years ago

On my list to take a look - if anyone else is poking around with this before me (few days at least) please update the bug with any findings and we can collaborate here.

Quuxplusone commented 3 years ago
Fascinating. Seems to happen in the frontend/IR generation..

With debug info (but -Xclang -disable-llvm-passes):

$_ZZ6createIsEDavE1I.1 = comdat any

$_ZZ6createIiEDavE1I.2 = comdat any

@_ZZ6createIsEDavE1I = external dso_local constant i32, align 4
@_ZZ6createIiEDavE1I = external dso_local constant i32, align 4
@_ZZ6createIsEDavE1I.1 = linkonce_odr dso_local constant i32 2, comdat, align
4, !dbg !0
@_ZZ6createIiEDavE1I.2 = linkonce_odr dso_local constant i32 2, comdat, align
4, !dbg !23

Compared to (without debug info):

$_ZZ6createIsEDavE1I = comdat any

$_ZZ6createIiEDavE1I = comdat any

@_ZZ6createIsEDavE1I = linkonce_odr dso_local constant i32 2, comdat, align 4
@_ZZ6createIiEDavE1I = linkonce_odr dso_local constant i32 2, comdat, align 4
Quuxplusone commented 3 years ago
So clang creates the GlobalVariable first at the behest of debug info emission
here:

llvm::GlobalVariable::GlobalVariable
      at llvm/lib/IR/Globals.cpp:365
clang::CodeGen::CodeGenModule::GetOrCreateLLVMGlobal
      at clang/lib/CodeGen/CodeGenModule.cpp:3712
clang::CodeGen::CodeGenModule::GetAddrOfGlobalVar
      at clang/lib/CodeGen/CodeGenModule.cpp:3928
clang::CodeGen::CGDebugInfo::CollectTemplateParams
      at clang/lib/CodeGen/CGDebugInfo.cpp:1904
clang::CodeGen::CGDebugInfo::CollectCXXTemplateParams
      at clang/lib/CodeGen/CGDebugInfo.cpp:2023
clang::CodeGen::CGDebugInfo::CreateLimitedType
      at clang/lib/CodeGen/CGDebugInfo.cpp:3415
clang::CodeGen::CGDebugInfo::getOrCreateLimitedType
      at clang/lib/CodeGen/CGDebugInfo.cpp:3322
clang::CodeGen::CGDebugInfo::CreateTypeDefinition
      at clang/lib/CodeGen/CGDebugInfo.cpp:2402
clang::CodeGen::CGDebugInfo::CreateType
      at clang/lib/CodeGen/CGDebugInfo.cpp:2387
clang::CodeGen::CGDebugInfo::CreateTypeNode
      at clang/lib/CodeGen/CGDebugInfo.cpp:3260
clang::CodeGen::CGDebugInfo::getOrCreateType
      at clang/lib/CodeGen/CGDebugInfo.cpp:3178
clang::CodeGen::CGDebugInfo::EmitDeclare
      at clang/lib/CodeGen/CGDebugInfo.cpp:4185
clang::CodeGen::CGDebugInfo::EmitDeclareOfAutoVariable
      at clang/lib/CodeGen/CGDebugInfo.cpp:4306
clang::CodeGen::CodeGenFunction::EmitAutoVarAlloca
      at clang/lib/CodeGen/CGDecl.cpp:1604
clang::CodeGen::CodeGenFunction::EmitAutoVarDecl
      at clang/lib/CodeGen/CGDecl.cpp:1308
clang::CodeGen::CodeGenFunction::EmitVarDecl
      at clang/lib/CodeGen/CGDecl.cpp:208
clang::CodeGen::CodeGenFunction::EmitDecl
      at clang/lib/CodeGen/CGDecl.cpp:153
clang::CodeGen::CodeGenFunction::EmitDeclStmt
      at clang/lib/CodeGen/CGStmt.cpp:1250
clang::CodeGen::CodeGenFunction::EmitSimpleStmt
      at clang/lib/CodeGen/CGStmt.cpp:386
clang::CodeGen::CodeGenFunction::EmitStmt
      at clang/lib/CodeGen/CGStmt.cpp:55
clang::CodeGen::CodeGenFunction::EmitCompoundStmtWithoutScope
      at clang/lib/CodeGen/CGStmt.cpp:476
clang::CodeGen::CodeGenFunction::EmitFunctionBody
      at clang/lib/CodeGen/CodeGenFunction.cpp:1181
clang::CodeGen::CodeGenFunction::GenerateCode
      at clang/lib/CodeGen/CodeGenFunction.cpp:1351
clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition
      at clang/lib/CodeGen/CodeGenModule.cpp:4731
clang::CodeGen::CodeGenModule::EmitGlobalDefinition
      at clang/lib/CodeGen/CodeGenModule.cpp:3081
clang::CodeGen::CodeGenModule::EmitGlobal
      at clang/lib/CodeGen/CodeGenModule.cpp:2833
clang::CodeGen::CodeGenModule::EmitTopLevelDecl
      at clang/lib/CodeGen/CodeGenModule.cpp:5546
#27 0x000000000a789442 in
      at clang/lib/CodeGen/ModuleBuilder.cpp:170
clang::BackendConsumer::HandleTopLevelDecl
      at clang/lib/CodeGen/CodeGenAction.cpp:218
clang::ParseAST
      at clang/lib/Parse/ParseAST.cpp:162
clang::ASTFrontendAction::ExecuteAction
      at clang/lib/Frontend/FrontendAction.cpp:1056
clang::CodeGenAction::ExecuteAction
      at clang/lib/CodeGen/CodeGenAction.cpp:1082
clang::FrontendAction::Execute
      at clang/lib/Frontend/FrontendAction.cpp:949
clang::CompilerInstance::ExecuteAction
      at clang/lib/Frontend/CompilerInstance.cpp:957
clang::ExecuteCompilerInvocation
      at clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:278
cc1_main
      at clang/tools/driver/cc1_main.cpp:240
ExecuteCC1Tool
      at clang/tools/driver/driver.cpp:330
main
      at clang/tools/driver/driver.cpp:407

And then seems to try to create it again at:

llvm::GlobalVariable::GlobalVariable
  at llvm/lib/IR/Globals.cpp:365
clang::CodeGen::CodeGenModule::getOrCreateStaticVarDecl
  at clang/lib/CodeGen/CGDecl.cpp:266
clang::CodeGen::CodeGenFunction::EmitStaticVarDecl
  at clang/lib/CodeGen/CGDecl.cpp:398
clang::CodeGen::CodeGenFunction::EmitVarDecl
  at clang/lib/CodeGen/CGDecl.cpp:201
clang::CodeGen::CodeGenFunction::EmitDecl
  at clang/lib/CodeGen/CGDecl.cpp:153
clang::CodeGen::CodeGenFunction::EmitDeclStmt
  at clang/lib/CodeGen/CGStmt.cpp:1250
clang::CodeGen::CodeGenFunction::EmitSimpleStmt
  at clang/lib/CodeGen/CGStmt.cpp:386
clang::CodeGen::CodeGenFunction::EmitStmt
  at clang/lib/CodeGen/CGStmt.cpp:55
clang::CodeGen::CodeGenFunction::EmitCompoundStmtWithoutScope
  at clang/lib/CodeGen/CGStmt.cpp:476
clang::CodeGen::CodeGenFunction::EmitFunctionBody
  at clang/lib/CodeGen/CodeGenFunction.cpp:1181
clang::CodeGen::CodeGenFunction::GenerateCode
  at clang/lib/CodeGen/CodeGenFunction.cpp:1351
clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition
  at clang/lib/CodeGen/CodeGenModule.cpp:4731
clang::CodeGen::CodeGenModule::EmitGlobalDefinition
  at clang/lib/CodeGen/CodeGenModule.cpp:3081
clang::CodeGen::CodeGenModule::EmitDeferred
  at clang/lib/CodeGen/CodeGenModule.cpp:2337
clang::CodeGen::CodeGenModule::Release
  at clang/lib/CodeGen/CodeGenModule.cpp:447
(anonymous namespace)::CodeGeneratorImpl::HandleTranslationUnit
  at clang/lib/CodeGen/ModuleBuilder.cpp:267
clang::BackendConsumer::HandleTranslationUnit
  at clang/lib/CodeGen/CodeGenAction.cpp:292
clang::ParseAST
  at clang/lib/Parse/ParseAST.cpp:171
clang::ASTFrontendAction::ExecuteAction
  at clang/lib/Frontend/FrontendAction.cpp:1056
clang::CodeGenAction::ExecuteAction
  at clang/lib/CodeGen/CodeGenAction.cpp:1082
clang::FrontendAction::Execute
  at clang/lib/Frontend/FrontendAction.cpp:949
clang::CompilerInstance::ExecuteAction
  at clang/lib/Frontend/CompilerInstance.cpp:957
clang::ExecuteCompilerInvocation
  at clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:278
cc1_main
  at clang/tools/driver/cc1_main.cpp:240
ExecuteCC1Tool
  at clang/tools/driver/driver.cpp:330
main
  at clang/tools/driver/driver.cpp:407

I haven't tested much of why this would be noteworthy for this new feature and
not in many other cases of debug info and C++ constructs - perhaps Richard
knows what might be at work here.

Very naively it looks like getOrCreateStaticVarDecl should be using
getOrCreateLLVMGlobal to ensure this sort of duplicate creation doesn't happen?