Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1151)

Issue 740023002: [NaCl SDK] Fix mysterious gtest failure in Html5FsTest::OpenForCreate (Closed)

Created:
6 years, 1 month ago by Sam Clegg
Modified:
6 years, 1 month ago
Reviewers:
binji
CC:
chromium-reviews, binji+watch_chromium.org, David Yen, Roland McGrath
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

[NaCl SDK] Fix mysterious gtest failure in Html5FsTest::OpenForCreate The failure seems to have been triggered from the recent gtest update. The exact revision seems to be: https://code.google.com/p/googletest/source/detail?r=693 The issue only effects builds made with our older gcc 4.4 toolchain. Neither asan nor valgrind report any issues with the linux run of this test, and PNaCl build also passes just fine so I'm assuming is a compiler bug in the older toolchain. The fix I found was to use "const char*" over "char []" for local string constants. In fact just adding a const alone and leaving the [] syntax also fixes the issue. Most likely there is an underlying compiler bug that still needs to be addressed. BUG=434821 Committed: https://crrev.com/ac5b4307414e447002ed67a038cad27264a5cce5 Cr-Commit-Position: refs/heads/master@{#304959}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -7 lines) Patch
M native_client_sdk/src/tests/nacl_io_test/html5_fs_test.cc View 6 chunks +7 lines, -7 lines 0 comments Download

Messages

Total messages: 12 (4 generated)
Sam Clegg
6 years, 1 month ago (2014-11-19 20:06:08 UTC) #2
binji
strange. lgtm
6 years, 1 month ago (2014-11-19 21:17:03 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/740023002/1
6 years, 1 month ago (2014-11-19 21:37:09 UTC) #5
Sam Clegg
Just saw some more errors just like this in naclports, so we probably need to ...
6 years, 1 month ago (2014-11-19 22:06:33 UTC) #7
Sam Clegg
On 2014/11/19 22:06:33, Sam Clegg wrote: > Just saw some more errors just like this ...
6 years, 1 month ago (2014-11-20 02:29:38 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/740023002/1
6 years, 1 month ago (2014-11-20 02:29:40 UTC) #10
commit-bot: I haz the power
Committed patchset #1 (id:1)
6 years, 1 month ago (2014-11-20 02:31:15 UTC) #11
commit-bot: I haz the power
6 years, 1 month ago (2014-11-20 02:31:48 UTC) #12
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/ac5b4307414e447002ed67a038cad27264a5cce5
Cr-Commit-Position: refs/heads/master@{#304959}

Powered by Google App Engine
This is Rietveld 408576698