This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Use GetNextInstructionPc in signal handlers
ClosedPublic

Authored by vitalybuka on Oct 1 2019, 6:58 PM.

Details

Summary

All other stack trace callers assume that PC contains return address.
HWAsan already use GetNextInstructionPc in similar code.

PR43339

Diff Detail

Repository
rL LLVM

Event Timeline

vitalybuka created this revision.Oct 1 2019, 6:58 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Herald added subscribers: llvm-commits, Restricted Project, dberris. · View Herald Transcript
vitalybuka updated this revision to Diff 222741.Oct 1 2019, 6:59 PM

clang-format

vitalybuka edited the summary of this revision. (Show Details)Oct 1 2019, 7:14 PM
vitalybuka updated this revision to Diff 222743.Oct 1 2019, 7:16 PM

Remove garbage comment

jmorse added a subscriber: jmorse.Oct 2 2019, 9:02 AM
eugenis accepted this revision.Oct 2 2019, 10:44 AM

LGTM

This revision is now accepted and ready to land.Oct 2 2019, 10:44 AM
eugenis requested changes to this revision.Oct 2 2019, 10:45 AM
eugenis added inline comments.
compiler-rt/test/sanitizer_common/TestCases/Linux/signal_line.cpp
9 ↗(On Diff #222743)

Well, this is not a good symptom.
Do you know why?

This revision now requires changes to proceed.Oct 2 2019, 10:45 AM
vitalybuka marked an inline comment as done.Oct 2 2019, 10:56 AM
vitalybuka added inline comments.
compiler-rt/test/sanitizer_common/TestCases/Linux/signal_line.cpp
9 ↗(On Diff #222743)

That's a copy/paste from similar test.
I'd like to land this one, if it's green the to try to land a patch removing this lines, for easier reverting

vitalybuka marked an inline comment as done.Oct 2 2019, 10:57 AM
vitalybuka added inline comments.
compiler-rt/test/sanitizer_common/TestCases/Linux/signal_line.cpp
9 ↗(On Diff #222743)

from sanitizer_common/TestCases/Linux/ill.cpp

remove REQUIRED

vitalybuka marked an inline comment as done.Oct 2 2019, 11:00 AM
vitalybuka added inline comments.
compiler-rt/test/sanitizer_common/TestCases/Linux/signal_line.cpp
9 ↗(On Diff #222743)

hm, probably safer to keep all platforms
at least we will know where it does not work ASAP
done

eugenis accepted this revision.Oct 2 2019, 11:01 AM
This revision is now accepted and ready to land.Oct 2 2019, 11:01 AM
This revision was automatically updated to reflect the committed changes.