This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer] Use architecture/slice information when symbolizing fat Mach-O files on Darwin
ClosedPublic

Authored by kubamracek on Dec 3 2016, 10:07 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

kubamracek updated this revision to Diff 80198.Dec 3 2016, 10:07 PM
kubamracek retitled this revision from to [sanitizer] Use architecture/slice information when symbolizing fat Mach-O files on Darwin.
kubamracek updated this object.
kubamracek set the repository for this revision to rL LLVM.
kubamracek added a project: Restricted Project.
kubamracek added a subscriber: llvm-commits.
filcab edited edge metadata.Dec 7 2016, 1:04 PM

This incremental change looks good. I'd rather let @kcc chime in in case he'd rather have it done another way, though.
But in the end, I'm not sure if this applies elsewhere, since most OS we support don't really have fat binaries.

lib/sanitizer_common/sanitizer_common.h
684 ↗(On Diff #80198)

I'm guessing you added this to two reviews so you commit either first but you'll merge the definitions later, right?

lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc
340 ↗(On Diff #80198)

Maybe return buf && ParseSymbolizePCOutput(buf, stack); instead of the if?
Same thing below.

test/asan/TestCases/Darwin/haswell-symbolication.cc
51 ↗(On Diff #80198)

Why?

kubamracek added inline comments.Dec 8 2016, 4:59 AM
lib/sanitizer_common/sanitizer_common.h
684 ↗(On Diff #80198)

Right.

lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc
340 ↗(On Diff #80198)

I think that would be less readable.

test/asan/TestCases/Darwin/haswell-symbolication.cc
51 ↗(On Diff #80198)

Yeah, the "REQUIRES: shell" doesn't make sense here. I misunderstood what it's used for.

filcab added inline comments.Dec 8 2016, 5:27 AM
lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc
340 ↗(On Diff #80198)

OK with me.

kubamracek updated this revision to Diff 81464.Dec 14 2016, 2:07 PM
kubamracek edited edge metadata.
kubamracek removed rL LLVM as the repository for this revision.

Updating patch, addressing review comments. Added comments that point to other places that have a list of architectures.

This revision was automatically updated to reflect the committed changes.