[clang] [Clang] Fix null pointer dereference in enum debug info generation (PR #97105)

[Tom Honermann via cfe-commits]

================
@@ -98,3 +98,6 @@ enum E8 { A8 = -128, B8 = 127 } x8;
 // CHECK-NOT: DIFlagEnumClass
 // CHECK: !DIEnumerator(name: "A8", value: -128)
 
+// Forward declaration of an enum class.
+enum class Color : int;
+// CHECK-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "Color"
----------------
tahonermann wrote:

Please follow the naming patterns already used in the test and rename the new enumeration to `E9`.

I added this change to my local build, but the test still passes without the associated change to `clang/lib/CodeGen/CGDebugInfo.cpp`. That suggests this test doesn't actually exercise the code in question.

`EnumDecl::getDefinition()` (see [here](https://github.com/llvm/llvm-project/blob/8d3252a8987818171878a26e4298b4b5dbf2a7e9/clang/include/clang/AST/Decl.h#L3943-L3945))  defers to `TagDecl::getDefinition()` (see [here](https://github.com/llvm/llvm-project/blob/8d3252a8987818171878a26e4298b4b5dbf2a7e9/clang/lib/AST/Decl.cpp#L4748-L4769)). The only way for the latter to return null is if `isCompleteDefinition()` (see [here](https://github.com/llvm/llvm-project/blob/8d3252a8987818171878a26e4298b4b5dbf2a7e9/clang/include/clang/AST/Decl.h#L3660)) is false for every declaration of the enumeration. For `isCompleteDefinition()` to return true, there has to have been an earlier call to `setCompleteDefinition()`. There are few calls to that function, the only relevant one looks to be from `TagDecl::completeDefinition()` (see [here](https://github.com/llvm/llvm-project/blob/8d3252a8987818171878a26e4298b4b5dbf2a7e9/clang/lib/AST/Decl.cpp#L4736-L4746)) by way of `EnumDecl::completeDefinition()` (see [here](https://github.com/llvm/llvm-project/blob/8d3252a8987818171878a26e4298b4b5dbf2a7e9/clang/lib/AST/Decl.cpp#L4866-L4877)). Auditing calls to `completeDefinition()` is a bit challenging, so I didn't check them all. The only obvious cases I see related to enumerations are calls related to when an enumeration definition is present. Based on that, I would expect the test to suffice to trigger a null dereference; unless `CGDebugInfo::CreateTypeDefinition()` is only called when a complete definition is available (in which case, the existing check for `isIncompleteType()` seems unnecessary and can be removed).

Can you do a bit more debugging to see if calls to `CGDebugInfo::CreateTypeDefinition(const EnumType *)` are in fact gated on a complete type definition? If they are, then the checks for `isIncompleteType()` can be removed and a non-null result from `ED->getDefinition()` can be asserted.

https://github.com/llvm/llvm-project/pull/97105