Add the win_apply_asan client script.

Add the win_apply_asan client script.

Meant to be used as a post-process step on the build output. Will take the
exe's and dll's in the output directory and will produce .asan.exe and .asan.dll
files (as well as the corresponding pdbs).

R=maruel@chromium.org


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=170491

[iannucci, 2012-11-28 03:21:23]

Cut out of https://chromiumcodereview.appspot.com/11379003/

[M-A Ruel, 2012-11-28 14:04:01]

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
File scripts/slave/chromium/win_apply_asan.py (right):

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:27: global OPTIONS, STOPPED
Instead, create an object and set them as member functions, it's much cleaner.

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:117: for pe_image in
(os.path.join(root, f) for f in files if GoodExeOrDll(f)):
What about foo.EXE ?

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:161: default_asan_dir = os.path.join(
You assume os.getcwd() makes sense here;

os.path.join(SCRIPT_DIR, '..', 'third_party', ...)
then you can os.path.normpath, or you can instead os.path.dirname(SCRIPT_DIR),
as you prefer. It's not strictly required.

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:192: options.full_directory =
os.path.join(options.build_dir, options.target)
I usually prefer to do:
options.build_dir = os.path.abspath(options.build_dir)
and any other flag that is a path, so that if cwd ever changes, you're not
running stuff from a random directory.

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:196: options.instrument_exe =
os.path.abspath(
So this way, you wouldn't have to do it at _utilization_ but instead at
_initialization_.

Note that it's preferable to call abspath() on existing path, hence it's better
to not call it here and on options.build_dir instead.

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:206: if args:
Put it above with the other checks, it's easier to grok.

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:211: UpdateAsanRuntime(options)
I often prefer to pass each argument individually now as I found it's easier to
manage on the long term when adding or removing arguments; e.g. you only pass
what is actually needed and not all the options. I don't mind much.

[iannucci, 2012-11-29 23:44:30]

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
File scripts/slave/chromium/win_apply_asan.py (right):

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:27: global OPTIONS, STOPPED
On 2012/11/28 14:04:02, Marc-Antoine Ruel wrote:
> Instead, create an object and set them as member functions, it's much cleaner.

That IS much cleaner. I'm not sure why I didn't do that to begin with.

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:117: for pe_image in
(os.path.join(root, f) for f in files if GoodExeOrDll(f)):
On 2012/11/28 14:04:02, Marc-Antoine Ruel wrote:
> What about foo.EXE ?

Done.

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:161: default_asan_dir = os.path.join(
On 2012/11/28 14:04:02, Marc-Antoine Ruel wrote:
> You assume os.getcwd() makes sense here;
> 
> os.path.join(SCRIPT_DIR, '..', 'third_party', ...)
> then you can os.path.normpath, or you can instead os.path.dirname(SCRIPT_DIR),
> as you prefer. It's not strictly required.

Done.

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:192: options.full_directory =
os.path.join(options.build_dir, options.target)
On 2012/11/28 14:04:02, Marc-Antoine Ruel wrote:
> I usually prefer to do:
> options.build_dir = os.path.abspath(options.build_dir)
> and any other flag that is a path, so that if cwd ever changes, you're not
> running stuff from a random directory.

Done.

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:206: if args:
On 2012/11/28 14:04:02, Marc-Antoine Ruel wrote:
> Put it above with the other checks, it's easier to grok.

Done.

https://chromiumcodereview.appspot.com/11428043/diff/2001/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:211: UpdateAsanRuntime(options)
On 2012/11/28 14:04:02, Marc-Antoine Ruel wrote:
> I often prefer to pass each argument individually now as I found it's easier
to
> manage on the long term when adding or removing arguments; e.g. you only pass
> what is actually needed and not all the options. I don't mind much.

Yeah, it's good practice. Otherwise options ends up being like an obfuscated
group of global variables. Done.

[M-A Ruel, 2012-11-29 23:52:59]

lgtm

https://chromiumcodereview.appspot.com/11428043/diff/6002/scripts/slave/chrom...
File scripts/slave/chromium/win_apply_asan.py (right):

https://chromiumcodereview.appspot.com/11428043/diff/6002/scripts/slave/chrom...
scripts/slave/chromium/win_apply_asan.py:130: stopped = multiprocessing.Event()
BTW, it'd probably be faster (especially latency-wise) to use a thread pool than
a process tool. Just saying, because it's probably not worth, it's way more code
since there's no stdlib equivalent to multiprocessing.Pool in threading. So the
potential performance gain could be not worth it if it's not a bottleneck, and I
suspect it's not.

[iannucci, 2012-11-30 01:28:18]

On 2012/11/29 23:52:59, Marc-Antoine Ruel wrote:
> lgtm
> 
>
https://chromiumcodereview.appspot.com/11428043/diff/6002/scripts/slave/chrom...
> File scripts/slave/chromium/win_apply_asan.py (right):
> 
>
https://chromiumcodereview.appspot.com/11428043/diff/6002/scripts/slave/chrom...
> scripts/slave/chromium/win_apply_asan.py:130: stopped =
multiprocessing.Event()
> BTW, it'd probably be faster (especially latency-wise) to use a thread pool
than
> a process tool. Just saying, because it's probably not worth, it's way more
code
> since there's no stdlib equivalent to multiprocessing.Pool in threading. So
the
> potential performance gain could be not worth it if it's not a bottleneck, and
I
> suspect it's not.

Yeah, I agree. I spent a good deal of time grumbling at the stdlib for not
having a mutithreading library (or a threading.Pool). I decided that the
additional code ugliness wasn't worth it. I also figured it wouldn't be too bad,
since multiprocessing.Pool reuses each process multiple times for multiple jobs.

FWIW, this asan step takes about 3 minutes on a z620. I would expect it to take
longer on a VM, but not that much longer (probably less than 15 min). I also
expect that I'll need to tune the --jobs parameter to not livelock everything on
the VMs due to IOPS shortage. The ASAN operation is pretty io-bound.

[commit-bot: I haz the power, 2012-11-30 18:28:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iannucci@chromium.org/11428043/6002

[commit-bot: I haz the power, 2012-11-30 18:33:13]

Change committed as 170491