This is an archive of the discontinued LLVM Phabricator instance.

Make asan_symbolize.py py3-compatible
ClosedPublic

Authored by chapuni on Dec 5 2016, 3:15 AM.

Details

Summary

This patch is squashed from my two private commits.

asan_symbolize.py: [Py3] Get rid of "print" statement. Use print() or write() instead.

Let me know if style issues anything.

asan_symbolize.py: [Py3] Use text mode with universal_newlines=True for Popen.

With universal_newlines, readline() stalls to fill the buffer. Therefore, let the pipe unbuffered.

FIXME: Use Popen.communicate()

ATM, I didn't touch their logic. I think they may be rewritten in future.

Tested both 2.7.6 and 3.4.3 on Ubuntu. Not tested on Windows yet.

Diff Detail

Repository
rL LLVM

Event Timeline

chapuni updated this revision to Diff 80241.Dec 5 2016, 3:15 AM
chapuni retitled this revision from to Make asan_symbolize.py py3-compatible.
chapuni updated this object.
chapuni added a reviewer: samsonov.
chapuni set the repository for this revision to rL LLVM.
chapuni added a subscriber: llvm-commits.
kcc edited reviewers, added: aizatsky; removed: samsonov.Jan 30 2017, 5:59 PM
kcc added a subscriber: kcc.

We want to stop using asan_symbolize.py in favor of the "sancov" tools.
If sancov works for you -- just use it.
If it does not, please tell us why.

asan_symbolize.py is used from check-asan tests. If you would like to obsolete it, I could apply this just to let tests pass before obsolete.

kcc added a comment.Jan 31 2017, 3:36 PM

asan_symbolize.py is used from check-asan tests. If you would like to obsolete it, I could apply this just to let tests pass before obsolete.

Of course, we'll do this before deleting asan_symbolize.py.
Do you need this change because some of the LLVM bots are starting to use Python3, or because you want to use asan_symbolize.py elesewhere?
If the latter, consider sancov instead.

@kcc, the former! check-asan is the last py3-incompatible one.

Please look into https://reviews.llvm.org/D27405 too!

mgorny added a subscriber: mgorny.Jan 31 2017, 11:41 PM
mgorny added inline comments.
compiler-rt/trunk/lib/asan/scripts/asan_symbolize.py
111

Not saying it's a problem but strictly speaking to preserve the original semantic, you could use:

print(symbolizer_input, file=self.pipe.stdin)

+ at the top:

from __future__ import print_function

(with the latter being a good idea anyway)

chapuni added inline comments.Feb 1 2017, 12:06 AM
compiler-rt/trunk/lib/asan/scripts/asan_symbolize.py
111

I was not noticed future when tweaking.

As Kostya says above, asan_symbolize will be obsoleted. I think it'd be trivial.

This revision was automatically updated to reflect the committed changes.