You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/02/12 23:28:32 UTC
[kudu-CR] logging: fix UBSAN unsigned int overflow in LogThrottler
Hello Dan Burkert,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/9289
to review the following change.
Change subject: logging: fix UBSAN unsigned int overflow in LogThrottler
......................................................................
logging: fix UBSAN unsigned int overflow in LogThrottler
Fixes the following UBSAN error:
src/kudu/util/logging.h:333:12: runtime error: unsigned integer
overflow: 563980051 - 563991872 cannot be represented in type 'unsigned
long'
This was happening because we used an unsigned int64 when subtracing
timestamps, and because this function is intentionally racy, it was
possible to underflow and end up negative.
No real functional issue fixed here since the worst that would happen
was an extra (non-throttled) log message when the race triggered.
Change-Id: Ib2078b5f49dc3c751b4bb7db893506494c758289
---
M src/kudu/util/logging.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/9289/1
--
To view, visit http://gerrit.cloudera.org:8080/9289
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib2078b5f49dc3c751b4bb7db893506494c758289
Gerrit-Change-Number: 9289
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
[kudu-CR] logging: fix UBSAN unsigned int overflow in LogThrottler
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/9289 )
Change subject: logging: fix UBSAN unsigned int overflow in LogThrottler
......................................................................
Removed reviewer Kudu Jenkins with the following votes:
* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/9289
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ib2078b5f49dc3c751b4bb7db893506494c758289
Gerrit-Change-Number: 9289
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
[kudu-CR] logging: fix UBSAN unsigned int overflow in LogThrottler
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9289 )
Change subject: logging: fix UBSAN unsigned int overflow in LogThrottler
......................................................................
Patch Set 1: Code-Review+2
looks like a flake
--
To view, visit http://gerrit.cloudera.org:8080/9289
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2078b5f49dc3c751b4bb7db893506494c758289
Gerrit-Change-Number: 9289
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 13 Feb 2018 00:10:37 +0000
Gerrit-HasComments: No
[kudu-CR] logging: fix UBSAN unsigned int overflow in LogThrottler
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9289 )
Change subject: logging: fix UBSAN unsigned int overflow in LogThrottler
......................................................................
logging: fix UBSAN unsigned int overflow in LogThrottler
Fixes the following UBSAN error:
src/kudu/util/logging.h:333:12: runtime error: unsigned integer
overflow: 563980051 - 563991872 cannot be represented in type 'unsigned
long'
This was happening because we used an unsigned int64 when subtracing
timestamps, and because this function is intentionally racy, it was
possible to underflow and end up negative.
No real functional issue fixed here since the worst that would happen
was an extra (non-throttled) log message when the race triggered.
Change-Id: Ib2078b5f49dc3c751b4bb7db893506494c758289
Reviewed-on: http://gerrit.cloudera.org:8080/9289
Reviewed-by: Dan Burkert <da...@cloudera.com>
Tested-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/util/logging.h
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Dan Burkert: Looks good to me, approved
Todd Lipcon: Verified
--
To view, visit http://gerrit.cloudera.org:8080/9289
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib2078b5f49dc3c751b4bb7db893506494c758289
Gerrit-Change-Number: 9289
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] logging: fix UBSAN unsigned int overflow in LogThrottler
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9289 )
Change subject: logging: fix UBSAN unsigned int overflow in LogThrottler
......................................................................
Patch Set 1: Verified+1
Indeed, I pinged Mike about it
--
To view, visit http://gerrit.cloudera.org:8080/9289
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2078b5f49dc3c751b4bb7db893506494c758289
Gerrit-Change-Number: 9289
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 13 Feb 2018 00:12:40 +0000
Gerrit-HasComments: No