You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2018/05/08 02:52:03 UTC
[kudu-CR] KUDU-2433. Don't try to symbolize a nullptr
Hello Alexey Serbin, Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/10341
to review the following change.
Change subject: KUDU-2433. Don't try to symbolize a nullptr
......................................................................
KUDU-2433. Don't try to symbolize a nullptr
Don't try to symbolize a nullptr when walking the stack. Even though a
null pc should be impossible, it might occur in cases where the stack
has been corrupted. Even though all bets are off in that case, if such
an error were to occur in a UBSAN-enabled build, we don't want to mask
the error while symbolizing a stack trace by crashing with a UBSAN
exception due to subtracting 1 from a null ptr, like what apparently
happened in KUDU-2433.
Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
---
M src/kudu/util/debug-util.cc
1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/10341/1
--
To view, visit http://gerrit.cloudera.org:8080/10341
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Gerrit-Change-Number: 10341
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
[kudu-CR] KUDU-2433. Don't try to symbolize a nullptr
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10341
to look at the new patch set (#2).
Change subject: KUDU-2433. Don't try to symbolize a nullptr
......................................................................
KUDU-2433. Don't try to symbolize a nullptr
Don't try to symbolize a nullptr when walking the stack. It may be
possible to get a null pc in the case of unwinding through a stack frame
in a library that wasn't built with frame pointers, or if the stack was
corrupted.
Attempting to symbolize a nullptr with this code path will overflow the
null pointer from 0 to UINT64_MAX, which causes a fatal exception in
UBSAN builds.
Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
---
M src/kudu/util/debug-util.cc
1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/10341/2
--
To view, visit http://gerrit.cloudera.org:8080/10341
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Gerrit-Change-Number: 10341
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2433. Don't try to symbolize a nullptr
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10341 )
Change subject: KUDU-2433. Don't try to symbolize a nullptr
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10341
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Gerrit-Change-Number: 10341
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 08 May 2018 18:21:37 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2433. Don't try to symbolize a nullptr
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has removed a vote on this change.
Change subject: KUDU-2433. Don't try to symbolize a nullptr
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10341
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Gerrit-Change-Number: 10341
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2433. Don't try to symbolize a nullptr
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10341 )
Change subject: KUDU-2433. Don't try to symbolize a nullptr
......................................................................
KUDU-2433. Don't try to symbolize a nullptr
Don't try to symbolize a nullptr when walking the stack. It may be
possible to get a null pc in the case of unwinding through a stack frame
in a library that wasn't built with frame pointers, or if the stack was
corrupted.
Attempting to symbolize a nullptr with this code path will overflow the
null pointer from 0 to UINT64_MAX, which causes a fatal exception in
UBSAN builds.
Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Reviewed-on: http://gerrit.cloudera.org:8080/10341
Reviewed-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Mike Percy <mp...@apache.org>
---
M src/kudu/util/debug-util.cc
1 file changed, 4 insertions(+), 1 deletion(-)
Approvals:
Todd Lipcon: Looks good to me, approved
Alexey Serbin: Looks good to me, approved
Mike Percy: Verified
--
To view, visit http://gerrit.cloudera.org:8080/10341
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Gerrit-Change-Number: 10341
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2433. Don't try to symbolize a nullptr
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10341 )
Change subject: KUDU-2433. Don't try to symbolize a nullptr
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10341
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Gerrit-Change-Number: 10341
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 08 May 2018 16:39:49 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2433. Don't try to symbolize a nullptr
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10341 )
Change subject: KUDU-2433. Don't try to symbolize a nullptr
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10341/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10341/1//COMMIT_MSG@11
PS1, Line 11: Even though all bets are off in that case
> per comment elsewhere, I think we sometimes get 00000000000 on the stack wh
Fixed, thanks for the good alternative explanation
http://gerrit.cloudera.org:8080/#/c/10341/1/src/kudu/util/debug-util.cc
File src/kudu/util/debug-util.cc:
http://gerrit.cloudera.org:8080/#/c/10341/1/src/kudu/util/debug-util.cc@700
PS1, Line 700: UB
> I think better to say "undefined behavior" instead of abbreviating.
Done
http://gerrit.cloudera.org:8080/#/c/10341/1/src/kudu/util/debug-util.cc@701
PS1, Line 701: corruption
> I think even without "corruption" per se, you can get 0 on the stack when b
Done
--
To view, visit http://gerrit.cloudera.org:8080/10341
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Gerrit-Change-Number: 10341
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 08 May 2018 16:38:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2433. Don't try to symbolize a nullptr
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10341 )
Change subject: KUDU-2433. Don't try to symbolize a nullptr
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10341/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10341/1//COMMIT_MSG@11
PS1, Line 11: Even though all bets are off in that case
per comment elsewhere, I think we sometimes get 00000000000 on the stack when unwinding through libc which doesn't have frame pointers. The stack is valid, it just relies on a fancier unwinder than we use.
http://gerrit.cloudera.org:8080/#/c/10341/1/src/kudu/util/debug-util.cc
File src/kudu/util/debug-util.cc:
http://gerrit.cloudera.org:8080/#/c/10341/1/src/kudu/util/debug-util.cc@700
PS1, Line 700: UB
I think better to say "undefined behavior" instead of abbreviating.
http://gerrit.cloudera.org:8080/#/c/10341/1/src/kudu/util/debug-util.cc@701
PS1, Line 701: corruption
I think even without "corruption" per se, you can get 0 on the stack when backtracing through libraries without frame pointers. I think you can just say "invalid unwinding" or somesuch
--
To view, visit http://gerrit.cloudera.org:8080/10341
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Gerrit-Change-Number: 10341
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 08 May 2018 15:20:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2433. Don't try to symbolize a nullptr
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10341 )
Change subject: KUDU-2433. Don't try to symbolize a nullptr
......................................................................
Patch Set 2: Verified+1
Overriding flaky test due to KUDU-2130
--
To view, visit http://gerrit.cloudera.org:8080/10341
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Gerrit-Change-Number: 10341
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 08 May 2018 19:22:43 +0000
Gerrit-HasComments: No