This is an archive of the discontinued LLVM Phabricator instance.

SanitizerCommon: fixes for unwinding & backtrace on SPARC
ClosedPublic

Authored by ebotcazou on Feb 20 2019, 2:53 AM.

Details

Summary

This patch contains various fixes for the unwinding and backtrace machinery on the SPARC, which doesn't work correctly in various cases. It was tested with GCC on SPARC/Solaris and SPARC/Linux.

Patch by Eric Botcazou.

Diff Detail

Repository
rL LLVM

Event Timeline

ebotcazou created this revision.Feb 20 2019, 2:53 AM
ro added a subscriber: ro.Feb 20 2019, 4:19 AM
vitalybuka added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_sparc.cc
35 ↗(On Diff #187536)

I am not sure that we need to compensate here
usually it's from GetStackTrace which pass the current PC or PC from signal

compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cc
139 ↗(On Diff #187536)

that's strange
is this know issue?

ebotcazou marked 2 inline comments as done.Feb 21 2019, 6:18 AM
ebotcazou added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_sparc.cc
35 ↗(On Diff #187536)

Yes, we do, the backtrace does GetPreviousInstructionPc in every case so, if you don' t do it there too, then the backtrace is wrong.

compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cc
139 ↗(On Diff #187536)

Yes, it's a known historical quirk on the SPARC. The root cause is probably that, in 32-bit mode, the return address is either %o7+8 or %o7+12 depending on the return type and the correct case isn't always easy to spot. That's not the case in 64-bit mode, but the usage was preserved, for the sake of consistency I presume.

Any objection to accepting this? We need it alongside D58433 to fix the Sanitizer failures in the GCC testsuite on the SPARC platforms and I'd rather not maintain it separately in the GCC tree.

vitalybuka accepted this revision.Feb 27 2019, 2:07 PM
This revision is now accepted and ready to land.Feb 27 2019, 2:07 PM
ebotcazou updated this revision to Diff 188684.Feb 28 2019, 1:30 AM
ebotcazou edited the summary of this revision. (Show Details)

Conflict in sanitizer_common/sanitizer_stacktrace_sparc.cc fixed.

Sorry for bothering you again, but yln's recent changes are in the way now.

ebotcazou updated this revision to Diff 188687.Feb 28 2019, 2:18 AM

Conflict in sanitizer_common/sanitizer_stacktrace_sparc.cc fixed.

Sorry for bothering you again, but yln's recent changes are in the way now.

Can anyone with write access install D5831, D5832, D5833 and D5834? Arcanist says that I don't have permission to push to the repository on GitHub. Thanks in advance.

Can anyone with write access install D58431, D58432, D58433 and D58434? Arcanist says that I don't have permission to push to the repository on GitHub. Thanks in advance.

vitalybuka edited the summary of this revision. (Show Details)Mar 12 2019, 11:19 AM
This revision was automatically updated to reflect the committed changes.