Page MenuHomePhabricator

[Analyzer] Do not segfault on unexpected call_once implementation
ClosedPublic

Authored by george.karpenkov on Oct 9 2017, 1:03 PM.

Details

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=34869

@dcoughlin Any advice on how to handle different stdlib implementations?
Can we conjure a separate symbol instead of relying on a particular struct layout?
For now this implementation will simply not go inside a differently implemented call_once.

Diff Detail

Repository
rL LLVM

Event Timeline

Did you link the correct bug in the description? The one you linked was closed long ago.

dcoughlin edited edge metadata.Oct 9 2017, 3:51 PM

@dcoughlin Any advice on how to handle different stdlib implementations?
Can we conjure a separate symbol instead of relying on a particular struct layout?
For now this implementation will simply not go inside a differently implemented call_once.

I think that for now your solution is the best to avoid the crashes. Let's see what Alexander has to say about the standard library causing the crashes. Ideally, we don't want to fall down too hard on libstdc++.

If we really need to handle a variety of standard libraries (or versions of standard libraries) we'll probably want to to treat std::call_once more abstractly and write a checker that models its behavior instead of body farming it.

lib/Analysis/BodyFarm.cpp
365 ↗(On Diff #118258)

LLVM style is to write this null check as if (!FlagCXXDecl).

369 ↗(On Diff #118258)

This return will leak the allocated AST nodes (as will the return for __state__ below). Can you hoist the validation checks to above the AST creation?

Review comments.

george.karpenkov marked 2 inline comments as done.Oct 9 2017, 4:14 PM
dcoughlin accepted this revision.Oct 9 2017, 4:19 PM

Looks good to me!

This revision is now accepted and ready to land.Oct 9 2017, 4:19 PM
This revision was automatically updated to reflect the committed changes.