Update the upstream version of asan_symbolize.py to r222535,

Update the upstream version of asan_symbolize.py to r222535,
which introduces support for the --dsym-hint option.

Add a dSYM hint producer to tools/valgrind/asan/asan_symbolize.py,
which builds paths to .dSYM files for executables and frameworks inside the
.app bundles, assuming the dSYM files are in the product dir.

BUG=148383, 242503, 170739, 166857
R=earthdok@chromium.org
NOTRY=true

Committed: https://crrev.com/7e52eba6cc9095bb535a2b8c85062e7f4262c48a
Cr-Commit-Position: refs/heads/master@{#308758}

[Alexander Potapenko, 2014-12-12 12:43:47]

TBR

[earthdok, 2014-12-12 16:03:14]

RSLGTM

[earthdok, 2014-12-12 16:14:04]

On 2014/12/12 16:03:14, earthdok wrote:
> RSLGTM

not lgtm following offline discussion on readability of
tools/valgrind/asan/asan_symbolize.py

[earthdok, 2014-12-12 16:22:45]

https://codereview.chromium.org/797253002/diff/1/tools/valgrind/asan/asan_sym...
File tools/valgrind/asan/asan_symbolize.py (right):

https://codereview.chromium.org/797253002/diff/1/tools/valgrind/asan/asan_sym...
tools/valgrind/asan/asan_symbolize.py:63: for index in range(len(parts)):
This can be expressed as "for (index, part) in enumerate(parts)".

But here we can do better: app_pos = next((i for i in xrange(len(parts)) if
parts[i].endswith('.app')), -1)

I would also use None instead of -1

[Alexander Potapenko, 2014-12-16 13:45:37]

PTAL

[earthdok, 2014-12-16 14:19:57]

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
File tools/valgrind/asan/asan_symbolize.py (right):

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
tools/valgrind/asan/asan_symbolize.py:59: parts = binary.split(os.path.sep)
I'd prefer a more descriptive name.

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
tools/valgrind/asan/asan_symbolize.py:62: for part, index in zip(parts,
range(len(parts))):
for index, part in enumerate(parts):

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
tools/valgrind/asan/asan_symbolize.py:67: # TODO(glider): more assertions
Why not add more assertions now?

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
tools/valgrind/asan/asan_symbolize.py:68: assert(len(framework_positions) < 2)
Please raise an exception, or at the very least assert with an error message.

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
tools/valgrind/asan/asan_symbolize.py:75: if len(bundles) > 1:
I don't understand what this part does.

[Alexander Potapenko, 2014-12-16 14:49:13]

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
File tools/valgrind/asan/asan_symbolize.py (right):

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
tools/valgrind/asan/asan_symbolize.py:59: parts = binary.split(os.path.sep)
On 2014/12/16 14:19:57, earthdok wrote:
> I'd prefer a more descriptive name.

Changed to "path_parts"

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
tools/valgrind/asan/asan_symbolize.py:62: for part, index in zip(parts,
range(len(parts))):
On 2014/12/16 14:19:57, earthdok wrote:
> for index, part in enumerate(parts):

Done.

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
tools/valgrind/asan/asan_symbolize.py:67: # TODO(glider): more assertions
On 2014/12/16 14:19:57, earthdok wrote:
> Why not add more assertions now?

Changed the second one, should be enough for now.

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
tools/valgrind/asan/asan_symbolize.py:68: assert(len(framework_positions) < 2)
On 2014/12/16 14:19:57, earthdok wrote:
> Please raise an exception, or at the very least assert with an error message.

Done.

https://codereview.chromium.org/797253002/diff/40001/tools/valgrind/asan/asan...
tools/valgrind/asan/asan_symbolize.py:75: if len(bundles) > 1:
On 2014/12/16 14:19:57, earthdok wrote:
> I don't understand what this part does.

Added a comment.

[earthdok, 2014-12-16 15:32:47]

https://codereview.chromium.org/797253002/diff/140001/tools/valgrind/asan/asa...
File tools/valgrind/asan/asan_symbolize.py (right):

https://codereview.chromium.org/797253002/diff/140001/tools/valgrind/asan/asa...
tools/valgrind/asan/asan_symbolize.py:80: if len(bundles) > 1:
Ok, how about something like this:

outermost_bundle = bundles[0]
product_dir = path_parts[:outermost_bundle]

# In case #2 this is the same as outermost_bundle.
innermost_bundle = bundles[-1]
dsym_path = product_dir + [path_parts[innermost_bundle]]

[Alexander Potapenko, 2014-12-16 17:18:46]

https://codereview.chromium.org/797253002/diff/140001/tools/valgrind/asan/asa...
File tools/valgrind/asan/asan_symbolize.py (right):

https://codereview.chromium.org/797253002/diff/140001/tools/valgrind/asan/asa...
tools/valgrind/asan/asan_symbolize.py:80: if len(bundles) > 1:
On 2014/12/16 15:32:47, earthdok wrote:
> Ok, how about something like this:
> 
> outermost_bundle = bundles[0]
> product_dir = path_parts[:outermost_bundle]
> 
> # In case #2 this is the same as outermost_bundle.
> innermost_bundle = bundles[-1]
> dsym_path = product_dir + [path_parts[innermost_bundle]]

Nice idea, thanks!
Done

[Alexander Potapenko, 2014-12-16 17:34:31]

On 2014/12/16 17:18:46, Alexander Potapenko wrote:
>
https://codereview.chromium.org/797253002/diff/140001/tools/valgrind/asan/asa...
> File tools/valgrind/asan/asan_symbolize.py (right):
> 
>
https://codereview.chromium.org/797253002/diff/140001/tools/valgrind/asan/asa...
> tools/valgrind/asan/asan_symbolize.py:80: if len(bundles) > 1:
> On 2014/12/16 15:32:47, earthdok wrote:
> > Ok, how about something like this:
> > 
> > outermost_bundle = bundles[0]
> > product_dir = path_parts[:outermost_bundle]
> > 
> > # In case #2 this is the same as outermost_bundle.
> > innermost_bundle = bundles[-1]
> > dsym_path = product_dir + [path_parts[innermost_bundle]]
> 
> Nice idea, thanks!
> Done

Added yet another assertion, now we're covered.

[earthdok, 2014-12-16 17:43:43]

https://codereview.chromium.org/797253002/diff/180001/tools/valgrind/asan/asa...
File tools/valgrind/asan/asan_symbolize.py (right):

https://codereview.chromium.org/797253002/diff/180001/tools/valgrind/asan/asa...
tools/valgrind/asan/asan_symbolize.py:68: assert len(framework_positions) < 2, \
<=1 for consistency

https://codereview.chromium.org/797253002/diff/180001/tools/valgrind/asan/asa...
tools/valgrind/asan/asan_symbolize.py:70: bundles = app_positions +
framework_positions
I think this should be named bundle_positions.

https://codereview.chromium.org/797253002/diff/180001/tools/valgrind/asan/asa...
tools/valgrind/asan/asan_symbolize.py:77: assert (len(bundles) == 1 or
Please add those assertions to the comment above.

[Alexander Potapenko, 2014-12-16 17:52:08]

https://codereview.chromium.org/797253002/diff/180001/tools/valgrind/asan/asa...
File tools/valgrind/asan/asan_symbolize.py (right):

https://codereview.chromium.org/797253002/diff/180001/tools/valgrind/asan/asa...
tools/valgrind/asan/asan_symbolize.py:68: assert len(framework_positions) < 2, \
On 2014/12/16 17:43:43, earthdok wrote:
> <=1 for consistency

Done.

https://codereview.chromium.org/797253002/diff/180001/tools/valgrind/asan/asa...
tools/valgrind/asan/asan_symbolize.py:70: bundles = app_positions +
framework_positions
On 2014/12/16 17:43:43, earthdok wrote:
> I think this should be named bundle_positions.

Done.

https://codereview.chromium.org/797253002/diff/180001/tools/valgrind/asan/asa...
tools/valgrind/asan/asan_symbolize.py:77: assert (len(bundles) == 1 or
On 2014/12/16 17:43:43, earthdok wrote:
> Please add those assertions to the comment above.

Done.

[earthdok, 2014-12-16 18:01:45]

lgtm

[Alexander Potapenko, 2014-12-17 08:02:38]

The CQ bit was checked by glider@chromium.org

[commit-bot: I haz the power, 2014-12-17 08:03:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/797253002/240001

[commit-bot: I haz the power, 2014-12-17 08:03:36]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2014-12-17 08:04:17]

Patchset 13 (id:??) landed as
https://crrev.com/7e52eba6cc9095bb535a2b8c85062e7f4262c48a
Cr-Commit-Position: refs/heads/master@{#308758}