This is an archive of the discontinued LLVM Phabricator instance.

Check for nullptr argument.
ClosedPublic

Authored by tra on May 10 2016, 4:14 PM.

Details

Summary

GetOrCreateLLVMGlobal() accepts nullptr D, but in some cases we end up dereferencing it without checking if it's non-null.

Fixes PR15492.

Diff Detail

Repository
rL LLVM

Event Timeline

tra updated this revision to Diff 56832.May 10 2016, 4:14 PM
tra retitled this revision from to Check for nullptr argument..
tra updated this object.
tra added reviewers: jlebar, jordan_rose.
tra added a subscriber: cfe-commits.
jlebar edited edge metadata.May 10 2016, 4:15 PM

Can we have a test?

tra added a comment.May 10 2016, 4:25 PM

I've never seen it triggered. Fix is based on the comment above the function that D==nullptr is acceptable and the fact that we are checking D in other places in this function.

Two cases where nullptr D is passed explicitly has something to do with -fblocks, but that does not seem to be used neither with CUDA nor with 'IsForDefinition' which was brought in to deal with a strange case of redefining C++ symbols in C. This is probably why it's not triggered.

If you have a check that doesn't have a test/is never triggered - simple
thing to do is just make it an assert & run some testing of your own
(selfhost, etc) then if there's insufficient evidence that it's needed,
ship it and wait until it fails on a buildbot, etc. Then reduce the test
case and check that in.

  • Dave
jlebar accepted this revision.May 10 2016, 5:14 PM
jlebar edited edge metadata.

OK, if the function explicitly says it accepts null values and if we check elsewhere in the function, I'm personally OK adding the checks.

This revision is now accepted and ready to land.May 10 2016, 5:14 PM
This revision was automatically updated to reflect the committed changes.