Page MenuHomePhabricator

Recompute invalidated iterator in insertTargetAndModeArgs
ClosedPublic

Authored by marcan on Mar 18 2018, 6:42 AM.

Details

Summary

Doing an .insert() can potentially invalidate iterators by reallocating the vector's storage. When all the stars align just right, this causes segfaults or glibc aborts. Fix this by recomputing the position before each insert.

Gentoo Linux bug (crashes while building Chromium): https://bugs.gentoo.org/650082

Diff Detail

Repository
rL LLVM

Event Timeline

marcan created this revision.Mar 18 2018, 6:42 AM

I'm not sure how to test this. Getting the bug to manifest involves all of having a lot of command line arguments, getting unlucky with the way SmallVectorImpl decides to allocate, and getting unlucky with the glibc allocator deciding to move the data block on realloc (or running under valgrind).

The easiest reproducer I have is valgrind x86_64-pc-linux-gnu-clang++ $(seq 1 512) with a debug build, but I'm not sure that that guarantees an abort in every case. 511 arguments is not sufficient to trigger it on my system, but 512 is.

mgorny added a subscriber: mgorny.Mar 18 2018, 10:03 AM

Good catch, thank you.

To avoid code duplication you could change InsertionPoint into an index and use begin() + InsertionPoint in calls to insert.

marcan updated this revision to Diff 138888.Mar 19 2018, 3:12 AM
sepavloff accepted this revision.Mar 19 2018, 6:12 AM

LGTM.

Thank you!

This revision is now accepted and ready to land.Mar 19 2018, 6:12 AM

Note that I don't have commit access, so someone else will have to commit it for me :)

This revision was automatically updated to reflect the committed changes.

Can this be backported to release_60 branch too please ?

Can this be backported to release_60 branch too please ?

The patch is compact, clear and addresses stability issue. I think it is worth of backporting to release_60.