This is an archive of the discontinued LLVM Phabricator instance.

Check for NULL Destination-Type when creating ArrayConstant
AcceptedPublic

Authored by bviyer on Jul 27 2018, 6:20 PM.

Details

Summary

While emitting Array Constant, if the destination type is null-pointer, it will cause an assert. This patch will check if the destination type is null, and if so then it will just return nullptr as the array constant (not something that is derived from destination type).
A test case is also attached.

Diff Detail

Repository
rC Clang

Event Timeline

bviyer created this revision.Jul 27 2018, 6:20 PM

I think the right fix here is to ensure that DestType is non-null at a higher level. Older branches of the compiler seem to be able to correctly emit this, probably because the initializer generated for this field ends up having type a[0]. Maybe we've just done some refactor that tries to optimize that but doesn't handle flexible array members correctly.

erik.pilkington added inline comments.
test/CodeGenCXX/empty-struct-init-list.cpp
2–5

You should add -emit-llvm, or else CodeGen won't even run. Also, expected-no-diagnostics only means anything when -verify is included on the run line.

bviyer updated this revision to Diff 158391.Jul 31 2018, 2:51 PM

Fixed code as suggested by John McCall.

bviyer updated this revision to Diff 158392.Jul 31 2018, 2:59 PM

Forgot to pass the -emit-llvm to the test case.

bviyer marked an inline comment as done.Jul 31 2018, 2:59 PM
rjmccall added inline comments.Aug 1 2018, 3:07 PM
test/CodeGenCXX/empty-struct-init-list.cpp
12

Please make this a FileCheck test that tests the actual compiler output.

bviyer updated this revision to Diff 158676.Aug 1 2018, 6:28 PM

Added FileCheck as requested by John McCall
Made comments start with "//" instead of /*
Removed expected-no-diagnostics from the test case (as requested by Erik)

bviyer marked an inline comment as done.Aug 1 2018, 6:31 PM
rjmccall added inline comments.Aug 3 2018, 6:14 PM
lib/CodeGen/CGExprConstant.cpp
2123

It definitely means that it's an incomplete array type, i.e. a flexible array member.

test/CodeGenCXX/empty-struct-init-list.cpp
12

I'm sorry, I should've been more specific. Please include a test line that shows the complete constant generated to initialize @d.

bviyer updated this revision to Diff 159353.Aug 6 2018, 12:34 PM
bviyer marked an inline comment as done.Aug 6 2018, 12:36 PM

John, I have updated the test case as you requested (I think). I am a bit new to Clang, so apologize if I mistook your request.

rjmccall accepted this revision.Aug 6 2018, 1:13 PM

Sure, that's fine.

This revision is now accepted and ready to land.Aug 6 2018, 1:13 PM