Patch re2 to support MSan.

Patch re2 to support MSan.

BUG=178409
R=eugenis@chromium.org
TBR=cpu@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=247998

[earthdok, 2014-01-29 23:00:56]

PTAL. Never patched third_party before - did I do it right?

[eugenis, 2014-01-30 09:02:55]

https://codereview.chromium.org/149723005/diff/1/third_party/re2/util/sparse_...
File third_party/re2/util/sparse_array.h (right):

https://codereview.chromium.org/149723005/diff/1/third_party/re2/util/sparse_...
third_party/re2/util/sparse_array.h:235: #if defined(MEMORY_SANITIZER)
I think it would be cleaner to implement RunningOnValgringOrMemorySanitizer() in
util/util.h and replace all calls to RunningOnValgrind with it.

[eugenis, 2014-01-30 09:03:41]

And please mention the upstream change and why we need to handle it differently
in chromium.

[earthdok, 2014-01-30 12:51:30]

done

[eugenis, 2014-01-30 13:11:11]

Patch at the below link adds one more location where memory must be initialized.
Could you check if this is needed here?
https://code.google.com/p/re2/issues/detail?id=77

[earthdok, 2014-01-30 13:26:09]

On 2014/01/30 13:11:11, eugenis wrote:
> Patch at the below link adds one more location where memory must be
initialized.
> Could you check if this is needed here?
> https://code.google.com/p/re2/issues/detail?id=77

Let's fix that in a separate patch.

[eugenis, 2014-01-30 13:27:10]

On 2014/01/30 13:26:09, earthdok wrote:
> On 2014/01/30 13:11:11, eugenis wrote:
> > Patch at the below link adds one more location where memory must be
> initialized.
> > Could you check if this is needed here?
> > https://code.google.com/p/re2/issues/detail?id=77
> 
> Let's fix that in a separate patch.

LGTM

[commit-bot: I haz the power, 2014-01-30 13:33:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/earthdok@chromium.org/149723005/20001

[commit-bot: I haz the power, 2014-01-30 18:11:23]

Change committed as 247998

[commit-bot: I haz the power, 2014-01-30 18:11:28]

CQ bit was unchecked on CL. Ignoring.

[Nico, 2014-02-06 17:26:02]

Thank you for checking that there is an upstream bug and for mentioning it in
the README change!

It might be worth to file a new issue for
https://code.google.com/p/re2/issues/detail?id=77#c2 given that that's just a
comment on a bug that's marked Fixed at the moment.

[eugenis, 2014-02-07 10:55:13]

https://code.google.com/p/re2/issues/detail?id=106

On Thu, Feb 6, 2014 at 9:26 PM,  <thakis@chromium.org> wrote:
> Thank you for checking that there is an upstream bug and for mentioning it
> in
> the README change!
>
> It might be worth to file a new issue for
> https://code.google.com/p/re2/issues/detail?id=77#c2 given that that's just
> a
> comment on a bug that's marked Fixed at the moment.
>
> https://codereview.chromium.org/149723005/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.