Page MenuHomePhabricator

[libclang] Fix crash when visiting a captured VLA.
ClosedPublic

Authored by ckandeler on Jun 26 2020, 1:46 AM.

Details

Summary

When a variable-length array is being captured in a lambda, the AST
contains two captures, the first one having a null init expression.
Visiting that one triggers an assertion, so skip it.

Diff Detail

Unit TestsFailed

TimeTest
10,420 mslinux > libomp.env::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -I /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test -I /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/llvm-project/build/lib -I /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test/ompt /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test/env/kmp_set_dispatch_buf.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/test/env/Output/kmp_set_dispatch_buf.c.tmp -lm -latomic && env KMP_DISP_NUM_BUFFERS=0 /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/test/env/Output/kmp_set_dispatch_buf.c.tmp
1,310 mslinux > libomp.worksharing/for::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -I /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test -I /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/llvm-project/build/lib -I /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test/ompt /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test/worksharing/for/kmp_set_dispatch_buf.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/test/worksharing/for/Output/kmp_set_dispatch_buf.c.tmp -lm -latomic && /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/test/worksharing/for/Output/kmp_set_dispatch_buf.c.tmp 7

Event Timeline

ckandeler created this revision.Jun 26 2020, 1:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 26 2020, 1:46 AM
ckandeler updated this revision to Diff 274071.Jun 29 2020, 5:25 AM

Fixed formatting issues.

this relates to https://reviews.llvm.org/D82740 too, but comes with a test which is good. personally I would place the check directly within Visit to make it more generically prevent a crash on invalid input, but that's probably a subjective design decision.

ckandeler added a comment.EditedJul 2 2020, 12:52 AM

I suppose it boils down to whether the visit() function is supposed to accept null arguments or not. Since I don't know the answer to that, I added the check at the caller. From the description of the linked patch, it appears that other places are also calling the function with a null pointer, but that could just mean that these should be fixed independently. Someone with better knowledge of the code would have to make that decision.

jkorous accepted this revision.Jul 6 2020, 4:11 PM

LGTM! Thanks for fixing this!

This revision is now accepted and ready to land.Jul 6 2020, 4:11 PM

@ckandeler do you have commit access or do you want me to land the patch?

@ckandeler do you have commit access or do you want me to land the patch?

I do not, so it'd be great if you could do it.

This revision was automatically updated to reflect the committed changes.