Toolchain crashes on interface redeclaration

[geoffromer]

Description of the bug:

The toolchain crashes if an interface is declared in an API file and redeclared in the corresponding impl file.

What did you do, or what's a simple way to reproduce the bug?

The following file_test input reproduces the issue:

// --- a.carbon

library "a";

interface I;

// --- a.impl.carbon

impl library "a";

interface I {}

What did you expect to happen?

I'm not sure whether this kind of thing should be valid, but I didn't expect it to crash.

What actually happened?

The compiler crashed.

Any other information, logs, or outputs that you want to share?

Here's the crash log:

[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from ToolchainFileTest
[ RUN      ] ToolchainFileTest.toolchain/check/testdata/interface/fail_no_definition_in_impl_file.carbon

To test this file alone, run:
  bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/interface/fail_no_definition_in_impl_file.carbon

Please report issues to https://github.com/carbon-language/carbon-lang/issues and include the crash backtrace.
Stack dump:
0.      Program arguments: /usr/local/google/home/gromer/.cache/bazel/_bazel_gromer/6b556be8ddbdbba287a2e1ef86f733d5/sandbox/linux-sandbox/7159/execroot/_main/bazel-out/k8-dbg/bin/toolchain/testing/file_test.runfiles/_main/toolchain/testing/file_test --test_targets_file=toolchain/te
sting/file_test.tests.txt --file_tests=toolchain/check/testdata/interface/fail_no_definition_in_impl_file.carbon
1.      bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/interface/fail_no_definition_in_impl_file.carbon
2.      Program arguments: compile --phase=check --dump-sem-ir --exclude-dump-file-prefix=/usr/local/google/home/gromer/.cache/bazel/_bazel_gromer/6b556be8ddbdbba287a2e1ef86f733d5/sandbox/linux-sandbox/7159/execroot/_main/bazel-out/k8-dbg/bin/toolchain/testing/file_test.runfiles/_ma
in/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/core a.carbon a.impl.carbon
3.      NodeStack:
        0.      InterfaceIntroducer -> no value
inst_block_stack_:
        0.      block<invalid>  {inst+0, inst+1, inst+2}
        1.      block<invalid>  {}
param_and_arg_refs_stack:
args_type_info_stack_:
4.      a.impl.carbon:4:1: Check::HandleInterfaceDefinitionStart
interface I {}
^~~~~~~~~~~~~
 #0 0x0000563addf2146d llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /proc/self/cwd/external/_main~llvm_project~llvm-project/llvm/lib/Support/Unix/Signals.inc:723:11
 #1 0x0000563addf219ab PrintStackTraceSignalHandler(void*) /proc/self/cwd/external/_main~llvm_project~llvm-project/llvm/lib/Support/Unix/Signals.inc:798:1
 #2 0x0000563addf1f086 llvm::sys::RunSignalHandlers() /proc/self/cwd/external/_main~llvm_project~llvm-project/llvm/lib/Support/Signals.cpp:105:5
 #3 0x0000563addf22375 SignalHandler(int) /proc/self/cwd/external/_main~llvm_project~llvm-project/llvm/lib/Support/Unix/Signals.inc:413:1
 #4 0x00007f8f882f9510 (/lib/x86_64-linux-gnu/libc.so.6+0x3c510)
 #5 0x00007f8f8834716c __pthread_kill_implementation ./nptl/./nptl/pthread_kill.c:44:76
 #6 0x00007f8f882f9472 raise ./signal/../sysdeps/posix/raise.c:27:6
 #7 0x00007f8f882e34b2 abort ./stdlib/./stdlib/abort.c:81:7
 #8 0x0000563add9ff145 (/usr/local/google/home/gromer/.cache/bazel/_bazel_gromer/6b556be8ddbdbba287a2e1ef86f733d5/sandbox/linux-sandbox/7159/execroot/_main/bazel-out/k8-dbg/bin/toolchain/testing/file_test.runfiles/_main/toolchain/testing/file_test+0x6cd8145)
 #9 0x0000563ad9c09995 (/usr/local/google/home/gromer/.cache/bazel/_bazel_gromer/6b556be8ddbdbba287a2e1ef86f733d5/sandbox/linux-sandbox/7159/execroot/_main/bazel-out/k8-dbg/bin/toolchain/testing/file_test.runfiles/_main/toolchain/testing/file_test+0x2ee2995)
#10 0x0000563adc9e7458 Carbon::SemIR::ConstantValueStore::Get(Carbon::SemIR::InstId) const /proc/self/cwd/./toolchain/sem_ir/constant.h:24:5
#11 0x0000563adcacfb5d Carbon::Check::ImportRefResolver::GetLocalConstantInstId(Carbon::SemIR::InstId) /proc/self/cwd/toolchain/check/import_ref.cpp:370:49
#12 0x0000563adcacd45d Carbon::Check::ImportRefResolver::TryResolveTypedInst(Carbon::SemIR::InterfaceDecl, Carbon::SemIR::ConstantId) /proc/self/cwd/toolchain/check/import_ref.cpp:1271:26
#13 0x0000563adcac9614 Carbon::Check::ImportRefResolver::TryResolveInst(Carbon::SemIR::InstId, Carbon::SemIR::ConstantId) /proc/self/cwd/toolchain/check/import_ref.cpp:697:16
#14 0x0000563adcac609e Carbon::Check::ImportRefResolver::Resolve(Carbon::SemIR::InstId) /proc/self/cwd/toolchain/check/import_ref.cpp:195:11
#15 0x0000563adcac5329 Carbon::Check::LoadImportRef(Carbon::Check::Context&, Carbon::SemIR::InstId) /proc/self/cwd/toolchain/check/import_ref.cpp:1522:31
#16 0x0000563adca6e5c0 Carbon::Check::Context::LookupNameInExactScope(Carbon::Check::SemIRLoc, Carbon::SemIR::NameId, Carbon::SemIR::NameScopeId, Carbon::SemIR::NameScope const&) /proc/self/cwd/toolchain/check/context.cpp:368:12
#17 0x0000563adca6e52e Carbon::Check::Context::LookupNameInDecl(Carbon::SemIR::LocId, Carbon::SemIR::NameId, Carbon::SemIR::NameScopeId) /proc/self/cwd/toolchain/check/context.cpp:247:12
#18 0x0000563adcaa5754 Carbon::Check::DeclNameStack::ApplyAndLookupName(Carbon::Check::DeclNameStack::NameContext&, Carbon::SemIR::LocId, Carbon::SemIR::NameId) /proc/self/cwd/toolchain/check/decl_name_stack.cpp:237:37
#19 0x0000563adcaa5a70 Carbon::Check::DeclNameStack::FinishName(Carbon::Check::NameComponent const&) /proc/self/cwd/toolchain/check/decl_name_stack.cpp:64:3
#20 0x0000563adca255a8 Carbon::Check::BuildInterfaceDecl(Carbon::Check::Context&, Carbon::Parse::NodeIdOneOf<Carbon::Parse::NodeIdForKind<Carbon::Parse::NodeKind::InterfaceDecl>, Carbon::Parse::NodeIdForKind<Carbon::Parse::NodeKind::InterfaceDefinitionStart> >) /proc/self/cwd/toolch
ain/check/handle_interface.cpp:33:3
#21 0x0000563adca25c4e Carbon::Check::HandleInterfaceDefinitionStart(Carbon::Check::Context&, Carbon::Parse::NodeIdForKind<Carbon::Parse::NodeKind::InterfaceDefinitionStart>) /proc/self/cwd/toolchain/check/handle_interface.cpp:117:44
#22 0x0000563adc9c2814 Carbon::Check::ProcessNodeIds(Carbon::Check::Context&, llvm::raw_ostream*, Carbon::ErrorTrackingDiagnosticConsumer&, Carbon::Parse::NodeLocConverter*) /proc/self/cwd/./toolchain/parse/node_kind.def:314:1
#23 0x0000563adc99c520 Carbon::Check::CheckParseTree(llvm::MutableArrayRef<Carbon::Parse::NodeLocConverter*>, Carbon::Check::(anonymous namespace)::UnitInfo&, int, llvm::raw_ostream*) /proc/self/cwd/toolchain/check/check.cpp:727:8
#24 0x0000563adc99a12d Carbon::Check::CheckParseTrees(llvm::MutableArrayRef<Carbon::Check::Unit>, bool, llvm::raw_ostream*) /proc/self/cwd/toolchain/check/check.cpp:1074:5
#25 0x0000563ad9e2a765 Carbon::Driver::Compile(Carbon::Driver::CompileOptions const&, Carbon::Driver::CodegenOptions const&) /proc/self/cwd/toolchain/driver/driver.cpp:871:3
#26 0x0000563ad9e29d36 Carbon::Driver::RunCommand(llvm::ArrayRef<llvm::StringRef>) /proc/self/cwd/toolchain/driver/driver.cpp:482:14
#27 0x0000563ad9c08790 Carbon::Testing::(anonymous namespace)::ToolchainFileTest::Run(llvm::SmallVector<llvm::StringRef, 3u> const&, llvm::vfs::InMemoryFileSystem&, llvm::raw_pwrite_stream&, llvm::raw_pwrite_stream&) /proc/self/cwd/toolchain/testing/file_test.cpp:45:33
#28 0x0000563ad9c2fa24 Carbon::Testing::FileTestBase::ProcessTestFileAndRun(Carbon::Testing::FileTestBase::TestContext&) /proc/self/cwd/testing/file_test/file_test_base.cpp:323:3
#29 0x0000563ad9c2d8ab Carbon::Testing::FileTestBase::TestBody() /proc/self/cwd/testing/file_test/file_test_base.cpp:138:21
#30 0x0000563ad9ccd09b void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /proc/self/cwd/external/googletest~/googletest/src/gtest.cc:2612:3
#31 0x0000563ad9c8e34a void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /proc/self/cwd/external/googletest~/googletest/src/gtest.cc:2648:14
#32 0x0000563ad9c8e293 testing::Test::Run() /proc/self/cwd/external/googletest~/googletest/src/gtest.cc:2694:3
#33 0x0000563ad9c8f282 testing::TestInfo::Run() /proc/self/cwd/external/googletest~/googletest/src/gtest.cc:2839:12
#34 0x0000563ad9c904ec testing::TestSuite::Run() /proc/self/cwd/external/googletest~/googletest/src/gtest.cc:3017:9
#35 0x0000563ad9ca063f testing::internal::UnitTestImpl::RunAllTests() /proc/self/cwd/external/googletest~/googletest/src/gtest.cc:5921:15
#36 0x0000563ad9cd1c3b bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/googletest~/googletest/src/gtest.cc:2612:3
#37 0x0000563ad9c9ffba bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/googletest~/googletest/src/gtest.cc:2648:14
#38 0x0000563ad9c9fde3 testing::UnitTest::Run() /proc/self/cwd/external/googletest~/googletest/src/gtest.cc:5484:10
#39 0x0000563ad9c499b1 RUN_ALL_TESTS() /proc/self/cwd/external/googletest~/googletest/include/gtest/gtest.h:2317:30
#40 0x0000563ad9c38968 Carbon::Testing::Main(int, char**) /proc/self/cwd/testing/file_test/file_test_base.cpp:932:12
#41 0x0000563ad9c37d42 main /proc/self/cwd/testing/file_test/file_test_base.cpp:939:3
#42 0x00007f8f882e46ca __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
#43 0x00007f8f882e4785 call_init ./csu/../csu/libc-start.c:128:20
#44 0x00007f8f882e4785 __libc_start_main ./csu/../csu/libc-start.c:347:5
#45 0x0000563ad9c064a1 _start (/usr/local/google/home/gromer/.cache/bazel/_bazel_gromer/6b556be8ddbdbba287a2e1ef86f733d5/sandbox/linux-sandbox/7159/execroot/_main/bazel-out/k8-dbg/bin/toolchain/testing/file_test.runfiles/_main/toolchain/testing/file_test+0x2edf4a1)
CHECK failure at ./toolchain/sem_ir/constant.h:24: inst_id.index >= 0

[gysddn]

I believe this is the same issue described in #4080, the problem here is not the redefining, rather the ImportRefResolver querying the interface's self_param instruction which is not defined because the interface itself is not defined.

The same error can be reproduced with a case even as simple as this:

// --- a.carbon

library "a";

interface I;

// --- a.impl.carbon

impl library "a";

var i: I;

The exact querying happens here:
https://github.com/carbon-language/carbon-lang/blob/trunk/toolchain/check/import_ref.cpp#L1332