Test for a module related crash in CGDebugInfo.cpp

[dyung]

Summary

In our internal testing, we found a crash in the compiler which was reduced to the attached test case. The issue was "fixed" a while back with Adrian's change in r248069 as the code causing the crash was changed and consequently no longer crashes. The crash occurred in the function CGDebugInfo::EmitImportDecl() in CGDebugInfo.cpp. We did not determine the underlying cause of the crash since it no longer occurs, but wanted to contribute the test case along with some info about it in case anyone might wish to investigate further in case the issue might still exist somewhere.

The function EmitImportDecl contained the following code:

void CGDebugInfo::EmitImportDecl(const ImportDecl &ID) {
    auto *Reader = CGM.getContext().getExternalSource();
    auto Info = Reader->getSourceDescriptor(*ID.getImportedModule());

The crash was because getExternalSource() was returning a null pointer which was assigned into Reader, and when the next line tried to use that pointer, it caused a crash.

[dyung]

Any thoughts on this test?

[dyung]

ping

[aprantl]

Thanks!

Is there anything meaningful that could be CHECKed here?
Usually a crash means that we previously had a code path without test coverage.

[dyung]

In D19048#414568, @aprantl wrote:

Thanks!

Is there anything meaningful that could be CHECKed here?
Usually a crash means that we previously had a code path without test coverage.

This was definitely a code path without test coverage.

When you ask whether there is anything meaningful that could be checked here, are you asking whether we can change the test from testing only that the compilation succeeds to checking something that the compiler produces? If so, I could change the test to produce llvm assembly (-S -emit-llvm) instead, and then perhaps check for the presence of a DIModule within the output. Would that make a better test?

[probinson]

test/Modules/getSourceDescriptor-crash.cpp

1 ↗

(On Diff #53515)

Should run %clang_cc1 not the driver.

[dyung]

Modified the test to emit llvm assembly and check for the presence of a DIModule in the output.

[dyung]

Change test to not use the driver.

[aprantl]

test/Modules/getSourceDescriptor-crash.cpp

6 ↗

(On Diff #55460)

This doesn't actually appear in the output :-)
You probably want to at least check for the correct name of the module and for the DIImportedEntity.

[dyung]

test/Modules/getSourceDescriptor-crash.cpp

6 ↗

(On Diff #55460)

Whoops! That was me trying to get it to fail to convince myself it was working and forgot to change it back. I'll restore it and modify the check to what you suggested. Thanks!

[dyung]

Fixed up test and check for that DIImportedEntity and DIModule are present in the generated output.

[aprantl]

Please make sure that the two DINodes are connected, otherwise LGTM. Thanks!

cfe/trunk/test/Modules/Inputs/getSourceDescriptor-crash/h1.h

+#pragma once

cfe/trunk/test/Modules/Inputs/getSourceDescriptor-crash/module.modulemap

+module foo {
+       header "h1.h"
+}

cfe/trunk/test/Modules/getSourceDescriptor-crash.cpp

+// RUN: %clang_cc1 -I %S/Inputs/getSourceDescriptor-crash -S -emit-llvm -debug-info-kind=limited -fimplicit-module-maps %s -o - | FileCheck %s
+
+#include "h1.h"
+#include "h1.h"
+
+// CHECK: DIImportedEntity
+// CHECK-SAME: entity: ![[ENTITY:[0-9]+]]
+// CHECK: ![[ENTITY]] = !DIModule
+// CHECK-SAME: name: "foo"