Page MenuHomePhabricator

[Clang][Driver] In -fintegrated-cc1 mode, avoid crashing on exit after a compiler crash
ClosedPublic

Authored by aganea on Mar 12 2020, 2:16 PM.

Details

Summary

As reported here: https://reviews.llvm.org/D69825#1916865 and in PR45164, in case of a compiler crash, Clang now reports a second crash related to TimerGroups. The crash is caused by the TimerGroup's internal linked list which still points to already freed stack frames, which were unwind by longjmp() (or SEH on Windows) after the first compiler crash.

There's a second crash, not seen in PR45164, which happens when llvm_shutdown is called: the static DefaultTimerGroup object is destroyed which causes more iterating through the TimerGroups.

With this patch, we now 'steal' the TimeGroup instance from DefaultTimerGroup and prevent any further iteration of the timers after the initial crash.

Diff Detail

Event Timeline

aganea created this revision.Mar 12 2020, 2:16 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 12 2020, 2:16 PM
rnk accepted this revision.Mar 12 2020, 5:47 PM

lgtm

The timer linked list is an interesting complication.

This revision is now accepted and ready to land.Mar 12 2020, 5:47 PM
hans accepted this revision.Mar 13 2020, 3:13 AM
This revision was automatically updated to reflect the committed changes.

Looks like there's a memory corruption detected by ASAN, but I'm not sure it's related to my change. Maybe someone with a more expert eye could tell?

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/26137/steps/check-asan%20in%20gcc%20build/logs/stdio

hans added a comment.Mar 13 2020, 8:31 AM

Looks like there's a memory corruption detected by ASAN, but I'm not sure it's related to my change. Maybe someone with a more expert eye could tell?

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/26137/steps/check-asan%20in%20gcc%20build/logs/stdio

The error seems to have scalar evolution (SCEV) on the stack, and there was a patch related to that in the same buildbot run (https://reviews.llvm.org/D70097), so I'm guessing that's the cause -- not your patch.