Page MenuHomePhabricator

Make sancov.py py3-compatible
ClosedPublic

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

Details

Summary

This patch is squashed from a few commits.

sancov.py: [Py3] Get rid of "print" statement. Use print() or write() instead.
sancov.py: [Py3] Use '//' instead of '/' as division operator.

Py3 emits float with '/'.

Py3 has '//' as integer division operator.

sancov.py: [Py3] Use sys.stdout.buffer for bytes.

Py2 doesn't have sys.stdout.buffer. An idea from http://stackoverflow.com/questions/23932332

sancov.py: [Py3] Use bytes.decode() explicitly.

Or bogus filename like "b'foo'" would be generated.

Diff Detail

Repository
rL LLVM

Event Timeline

chapuni updated this revision to Diff 80243.Dec 5 2016, 3:31 AM
chapuni retitled this revision from to Make sancov.py py3-compatible.
chapuni updated this object.
chapuni added a reviewer: kcc.
chapuni set the repository for this revision to rL LLVM.
chapuni added a subscriber: llvm-commits.
kcc edited edge metadata.

ok, makes sense. I'll let Mike review this change.
Thanks!

mgorny added a subscriber: mgorny.Jan 31 2017, 11:48 PM
mgorny added inline comments.
compiler-rt/trunk/lib/sanitizer_common/scripts/sancov.py
73–76

That's Python 2.7+ but I guess we don't have to care about 2.6 anymore.

140

I think you ate a newline here.

223

Newline?

chapuni marked 2 inline comments as done.Feb 1 2017, 12:13 AM

@mgorny Thanks to review.

compiler-rt/trunk/lib/sanitizer_common/scripts/sancov.py
73–76
140

Yes. I missed since check-asan didn't complain.

223

It ought to emit a newline.

mgorny added inline comments.Feb 1 2017, 12:34 AM
compiler-rt/trunk/lib/sanitizer_common/scripts/sancov.py
223

You are correct. My mistake, sorry.

chapuni marked an inline comment as done.Feb 1 2017, 12:37 AM
chapuni added inline comments.
compiler-rt/trunk/lib/sanitizer_common/scripts/sancov.py
93

I will add \n here later, too.

aizatsky edited edge metadata.Feb 1 2017, 9:52 AM

Does this script actually run with python3 after these changes, or are these lint-driven errors? I'm not an expert in python3 (only used python2), but afaik it doesn't have % operator (you are supposed to use "".format()).

@aizatsky, I didn't know operator '%' will be obsoleted in future. I will rewrite them with format().

I am familiar to printf-like formatter, though.

Could you please clarify if you test it with python3 and if it works?

@aizatsky, Yes, my patches aim to check-asan passing on py3.
I wrote such a clarification to https://reviews.llvm.org/D27404
but I forgot to write one here during creating diffs.

Tested on x86_64-ubuntu.

trunk,

Failing Tests (30):
    AddressSanitizer-i386-linux :: TestCases/Linux/coverage-missing.cc
    AddressSanitizer-i386-linux :: TestCases/Posix/asan-symbolize-bad-path.cc
    AddressSanitizer-i386-linux :: TestCases/Posix/asan-symbolize-sanity-test.cc
    AddressSanitizer-i386-linux :: TestCases/Posix/coverage-direct-activation.cc
    AddressSanitizer-i386-linux :: TestCases/Posix/coverage-direct-large.cc
    AddressSanitizer-i386-linux :: TestCases/Posix/coverage-direct.cc
    AddressSanitizer-i386-linux :: TestCases/Posix/coverage-fork-direct.cc
    AddressSanitizer-i386-linux :: TestCases/Posix/coverage-sandboxing.cc
    AddressSanitizer-i386-linux :: TestCases/Posix/coverage.cc
    AddressSanitizer-i386-linux :: TestCases/coverage-and-lsan.cc
    AddressSanitizer-i686-linux :: TestCases/Linux/coverage-missing.cc
    AddressSanitizer-i686-linux :: TestCases/Posix/asan-symbolize-bad-path.cc
    AddressSanitizer-i686-linux :: TestCases/Posix/asan-symbolize-sanity-test.cc
    AddressSanitizer-i686-linux :: TestCases/Posix/coverage-direct-activation.cc
    AddressSanitizer-i686-linux :: TestCases/Posix/coverage-direct-large.cc
    AddressSanitizer-i686-linux :: TestCases/Posix/coverage-direct.cc
    AddressSanitizer-i686-linux :: TestCases/Posix/coverage-fork-direct.cc
    AddressSanitizer-i686-linux :: TestCases/Posix/coverage-sandboxing.cc
    AddressSanitizer-i686-linux :: TestCases/Posix/coverage.cc
    AddressSanitizer-x86_64-linux :: TestCases/Linux/coverage-missing.cc
    AddressSanitizer-x86_64-linux :: TestCases/Posix/asan-symbolize-bad-path.cc
    AddressSanitizer-x86_64-linux :: TestCases/Posix/asan-symbolize-sanity-test.cc
    AddressSanitizer-x86_64-linux :: TestCases/Posix/coverage-direct-activation.cc
    AddressSanitizer-x86_64-linux :: TestCases/Posix/coverage-direct-large.cc
    AddressSanitizer-x86_64-linux :: TestCases/Posix/coverage-direct.cc
    AddressSanitizer-x86_64-linux :: TestCases/Posix/coverage-fork-direct.cc
    AddressSanitizer-x86_64-linux :: TestCases/Posix/coverage-sandboxing.cc
    AddressSanitizer-x86_64-linux :: TestCases/Posix/coverage.cc
    AddressSanitizer-x86_64-linux :: TestCases/coverage-and-lsan.cc
    AddressSanitizer-x86_64-linux :: TestCases/coverage-order-pcs.cc

  Expected Passes    : 1411
  Expected Failures  : 3
  Unsupported Tests  : 443
  Unexpected Failures: 30

With patches,

Testing Time: 52.02s
  Expected Passes    : 1441
  Expected Failures  : 3
  Unsupported Tests  : 443

If this script working under python3 is your goal, then I suggest you make the script work first, rather then try to fix lint issues separately. I'd rather we do python3-ification in one change if possible.

@aizatsky, Yes, I can let sancov (and symbolize) work on both py2 and py3. May I commit changes (and pull them into release_40)?

After my investigation, String.format() is just *strongly suggested*.

aizatsky accepted this revision.Feb 6 2017, 6:25 PM

LG then.

This revision is now accepted and ready to land.Feb 6 2017, 6:25 PM
This revision was automatically updated to reflect the committed changes.

Thanks @aizatsky, check-asan is py3-compatible now.