Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2591023004:
Fix use-after-scope bug in TextElider. (Closed)
DescriptionFix use-after-scope bug in TextElider.
string.c_str() result was implicitly converted to icu::UnicodeString,
a reference to the temp variable was saved (and then used) by
icu::BreakIterator at the time when the stack address was already
potentially being reused for other local variables.
The bug was found by AddressSanitizer with use-after-free check
enabled. It's currently being rolled out into Chrome, and this CL
is a part of a larger cleanup of existing failures.
BUG=649897
Committed: https://crrev.com/ebc675de6100792906c33c0e0be5ba8c78940f3d
Cr-Commit-Position: refs/heads/master@{#440263}
Patch Set 1 #Patch Set 2 : Fix use-after-scope bug in TextElider. #
Messages
Total messages: 17 (9 generated)
krasin@chromium.org changed reviewers: + msw@chromium.org
The CQ bit was checked by krasin@chromium.org to run a CQ dry run
Description was changed from ========== Fix use-after-scope bug in TextElider. string.c_str() was implicitly converted to icu::UnicodeString, a reference to the temp variable was saved (and then used) by icu::BreakIterator at the time when the stack address was already potentially being reused for other local variables. The bug was found by AddressSanitizer with use-after-free check enabled. It's currently being rolled out into Chrome, and this CL is a part of a larger cleanup of existing failures. BUG=649897 ========== to ========== Fix use-after-scope bug in TextElider. string.c_str() result was implicitly converted to icu::UnicodeString, a reference to the temp variable was saved (and then used) by icu::BreakIterator at the time when the stack address was already potentially being reused for other local variables. The bug was found by AddressSanitizer with use-after-free check enabled. It's currently being rolled out into Chrome, and this CL is a part of a larger cleanup of existing failures. BUG=649897 ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm; but this code should probably be using base/i18n/break_iterator.h
The CQ bit was unchecked by krasin@chromium.org
The CQ bit was checked by krasin@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/12/21 21:48:19, msw wrote: > lgtm; but this code should probably be using base/i18n/break_iterator.h I am not against doing this cleanup in a separate CL, but I will need some guidance, as my work on Chromium is not concentrated on specific parts, and it means I am lucking the knowledge of the level icu vs base break iterators.
On 2016/12/21 21:55:42, krasin1 wrote: > On 2016/12/21 21:48:19, msw wrote: > > lgtm; but this code should probably be using base/i18n/break_iterator.h > > I am not against doing this cleanup in a separate CL, but I will need some > guidance, as my work on Chromium is not concentrated on specific parts, and it > means I am lucking the knowledge of the level icu vs base break iterators. And thanks for the instant review!
On 2016/12/21 21:55:42, krasin1 wrote: > On 2016/12/21 21:48:19, msw wrote: > > lgtm; but this code should probably be using base/i18n/break_iterator.h > > I am not against doing this cleanup in a separate CL, but I will need some > guidance, as my work on Chromium is not concentrated on specific parts, and it > means I am lucking the knowledge of the level icu vs base break iterators. No worries, you need not follow up; it was more of an aside comment.
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482357236136640,
"parent_rev": "4b42dcec00bdc90a94105e7221d468a04c61e4e0", "commit_rev":
"283e02b1b68eeb0317b4fe38982c60b33e7d4833"}
Message was sent while issue was closed.
Description was changed from ========== Fix use-after-scope bug in TextElider. string.c_str() result was implicitly converted to icu::UnicodeString, a reference to the temp variable was saved (and then used) by icu::BreakIterator at the time when the stack address was already potentially being reused for other local variables. The bug was found by AddressSanitizer with use-after-free check enabled. It's currently being rolled out into Chrome, and this CL is a part of a larger cleanup of existing failures. BUG=649897 ========== to ========== Fix use-after-scope bug in TextElider. string.c_str() result was implicitly converted to icu::UnicodeString, a reference to the temp variable was saved (and then used) by icu::BreakIterator at the time when the stack address was already potentially being reused for other local variables. The bug was found by AddressSanitizer with use-after-free check enabled. It's currently being rolled out into Chrome, and this CL is a part of a larger cleanup of existing failures. BUG=649897 Review-Url: https://codereview.chromium.org/2591023004 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Fix use-after-scope bug in TextElider. string.c_str() result was implicitly converted to icu::UnicodeString, a reference to the temp variable was saved (and then used) by icu::BreakIterator at the time when the stack address was already potentially being reused for other local variables. The bug was found by AddressSanitizer with use-after-free check enabled. It's currently being rolled out into Chrome, and this CL is a part of a larger cleanup of existing failures. BUG=649897 Review-Url: https://codereview.chromium.org/2591023004 ========== to ========== Fix use-after-scope bug in TextElider. string.c_str() result was implicitly converted to icu::UnicodeString, a reference to the temp variable was saved (and then used) by icu::BreakIterator at the time when the stack address was already potentially being reused for other local variables. The bug was found by AddressSanitizer with use-after-free check enabled. It's currently being rolled out into Chrome, and this CL is a part of a larger cleanup of existing failures. BUG=649897 Committed: https://crrev.com/ebc675de6100792906c33c0e0be5ba8c78940f3d Cr-Commit-Position: refs/heads/master@{#440263} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/ebc675de6100792906c33c0e0be5ba8c78940f3d Cr-Commit-Position: refs/heads/master@{#440263} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
