You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Jim Apple (Code Review)" <ge...@cloudera.org> on 2017/06/11 17:07:52 UTC

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool. 

Jim Apple has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7148

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................

IMPALA-5031: Don't dereference uninitialized memory as a bool.

Every bool takes up a byte of memory, but not every byte of memory is
a valid bool. Dereferencing that memory, even if only to write to, it
is undefined behavior.

Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Testing: running exhaustive UBSAN and DEBUG.
---
M be/src/runtime/raw-value.cc
1 file changed, 5 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/7148/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool.

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has uploaded a new patch set (#2).

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................

IMPALA-5031: Don't dereference uninitialized memory as a bool.

Every bool takes up a byte of memory, but not every byte of memory is
a valid bool. Dereferencing that memory, even if only to write to, it
is undefined behavior.

I tested exhaustive UBSAN and DEBUG. In UBSAN, the error is no longer
present; in both UBSAN and DEBUG, all tests pass

Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
---
M be/src/runtime/raw-value.cc
1 file changed, 5 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/7148/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool.

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................


Patch Set 3:

> Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/720/

https://issues.apache.org/jira/browse/IMPALA-3040, trying again.

-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................


Patch Set 3:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/720/

-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................


Patch Set 3: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................


Patch Set 3: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/720/

-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................


IMPALA-5031: Don't dereference uninitialized memory as a bool.

Every bool takes up a byte of memory, but not every byte of memory is
a valid bool. Dereferencing that memory, even if only to write to, it
is undefined behavior.

I tested exhaustive UBSAN and DEBUG. In UBSAN, the error is no longer
present; in both UBSAN and DEBUG, all tests pass

Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Reviewed-on: http://gerrit.cloudera.org:8080/7148
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Impala Public Jenkins
---
M be/src/runtime/raw-value.cc
1 file changed, 5 insertions(+), 1 deletion(-)

Approvals:
  Impala Public Jenkins: Verified
  Jim Apple: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool.

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................


Patch Set 2: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................


Patch Set 3:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/722/

-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool.

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................


Patch Set 3: Code-Review+2

rebase carry

-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Don't dereference uninitialized memory as a bool.

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-5031: Don't dereference uninitialized memory as a bool.
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7148/1/be/src/runtime/raw-value.cc
File be/src/runtime/raw-value.cc:

Line 120
Example backtrace:

    /home/ubuntu/Impala/be/src/runtime/raw-value.cc:120:39: runtime error: load of value 197, which is not a valid value for type 'const bool'
    #0 0x7f71b6a6ccef in impala::RawValue::Write(void const*, void*, impala::ColumnType const&, impala::MemPool*) /home/ubuntu/Impala/be/src/runtime/raw-value.cc:120:39
    #1 0x7f71b3b47e20 in impala::HashTableCtx::EvalRow(impala::TupleRow const*, std::vector<impala::ExprContext*, std::allocator<impala::ExprContext*> > const&, unsigned char*, unsigned char*) /home/ubuntu/Impala/be/src/exec/hash-table.cc:181:5
    #2 0x7f71b42b9bb7 in impala::HashTableCtx::EvalProbeRow(impala::TupleRow const*, unsigned char*, unsigned char*) /home/ubuntu/Impala/be/src/exec/hash-table.h:428:12
    #3 0x7f71b42b9665 in impala::HashTableCtx::EvalAndHashProbe(impala::TupleRow const*) /home/ubuntu/Impala/be/src/exec/hash-table.inline.h:41:19
    #4 0x7f71b42ad473 in void impala::PartitionedAggregationNode::EvalAndHashPrefetchGroup<false>(impala::RowBatch*, int, impala::TPrefetchMode::type, impala::HashTableCtx*) /home/ubuntu/Impala/be/src/exec/partitioned-aggregation-node-ir.cc:83:18
    #5 0x7f71b42a898f in impala::PartitionedAggregationNode::ProcessBatchStreaming(bool, impala::TPrefetchMode::type, impala::RowBatch*, impala::RowBatch*, impala::HashTableCtx*, int*) /home/ubuntu/Impala/be/src/exec/partitioned-aggregation-node-ir.cc:190:5
    #6 0x7f71b4243fa2 in impala::PartitionedAggregationNode::GetRowsStreaming(impala::RuntimeState*, impala::RowBatch*) /home/ubuntu/Impala/be/src/exec/partitioned-aggregation-node.cc:584:33
    #7 0x7f71b423af36 in impala::PartitionedAggregationNode::GetNextInternal(impala::RuntimeState*, impala::RowBatch*, bool*) /home/ubuntu/Impala/be/src/exec/partitioned-aggregation-node.cc:451:31
    #8 0x7f71b42395ef in impala::PartitionedAggregationNode::GetNext(impala::RuntimeState*, impala::RowBatch*, bool*) /home/ubuntu/Impala/be/src/exec/partitioned-aggregation-node.cc:376:29
    #9 0x7f71b6903b11 in impala::FragmentInstanceState::ExecInternal() /home/ubuntu/Impala/be/src/runtime/fragment-instance-state.cc:270:33
    #10 0x7f71b68f695e in impala::FragmentInstanceState::Exec() /home/ubuntu/Impala/be/src/runtime/fragment-instance-state.cc:88:14
    #11 0x7f71b69d6f6e in impala::QueryState::ExecFInstance(impala::FragmentInstanceState*) /home/ubuntu/Impala/be/src/runtime/query-state.cc:337:19
    #12 0x7f71b6a224f0 in boost::_mfi::mf1<void, impala::QueryState, impala::FragmentInstanceState*>::operator()(impala::QueryState*, impala::FragmentInstanceState*) const /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/mem_fn_template.hpp:165:16
    #13 0x7f71b6a22348 in void boost::_bi::list2<boost::_bi::value<impala::QueryState*>, boost::_bi::value<impala::FragmentInstanceState*> >::operator()<boost::_mfi::mf1<void, impala::QueryState, impala::FragmentInstanceState*>, boost::_bi::list0>(boost::_bi::type<void>, boost::_mfi::mf1<void, impala::QueryState, impala::FragmentInstanceState*>&, boost::_bi::list0&, int) /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind.hpp:313:9
    #14 0x7f71b6a21ffb in boost::_bi::bind_t<void, boost::_mfi::mf1<void, impala::QueryState, impala::FragmentInstanceState*>, boost::_bi::list2<boost::_bi::value<impala::QueryState*>, boost::_bi::value<impala::FragmentInstanceState*> > >::operator()() /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind_template.hpp:20:16
    #15 0x7f71b6a213d9 in boost::detail::function::void_function_obj_invoker0<boost::_bi::bind_t<void, boost::_mfi::mf1<void, impala::QueryState, impala::FragmentInstanceState*>, boost::_bi::list2<boost::_bi::value<impala::QueryState*>, boost::_bi::value<impala::FragmentInstanceState*> > >, void>::invoke(boost::detail::function::function_buffer&) /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/function/function_template.hpp:153:11
    #16 0x7f71b864f564 in boost::function0<void>::operator()() const /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/function/function_template.hpp:766:14
    #17 0x7f71b86406a0 in impala::Thread::SuperviseThread(std::string const&, std::string const&, boost::function<void ()>, impala::Promise<long>*) /home/ubuntu/Impala/be/src/util/thread.cc:325:3
    #18 0x7f71b8668713 in void boost::_bi::list4<boost::_bi::value<std::string>, boost::_bi::value<std::string>, boost::_bi::value<boost::function<void ()> >, boost::_bi::value<impala::Promise<long>*> >::operator()<void (*)(std::string const&, std::string const&, boost::function<void ()>, impala::Promise<long>*), boost::_bi::list0>(boost::_bi::type<void>, void (*&)(std::string const&, std::string const&, boost::function<void ()>, impala::Promise<long>*), boost::_bi::list0&, int) /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind.hpp:457:9
    #19 0x7f71b866803b in boost::_bi::bind_t<void, void (*)(std::string const&, std::string const&, boost::function<void ()>, impala::Promise<long>*), boost::_bi::list4<boost::_bi::value<std::string>, boost::_bi::value<std::string>, boost::_bi::value<boost::function<void ()> >, boost::_bi::value<impala::Promise<long>*> > >::operator()() /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind_template.hpp:20:16
    #20 0x7f71b8666a85 in boost::detail::thread_data<boost::_bi::bind_t<void, void (*)(std::string const&, std::string const&, boost::function<void ()>, impala::Promise<long>*), boost::_bi::list4<boost::_bi::value<std::string>, boost::_bi::value<std::string>, boost::_bi::value<boost::function<void ()> >, boost::_bi::value<impala::Promise<long>*> > > >::run() /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/thread/detail/thread.hpp:116:17
    #21 0xa460b9 in thread_proxy (/home/ubuntu/Impala/be/build/debug/service/impalad+0xa460b9)
    #22 0x7f71a8c53183 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8183)
    #23 0x7f71a898037c in clone (/lib/x86_64-linux-gnu/libc.so.6+0xfa37c)


-- 
To view, visit http://gerrit.cloudera.org:8080/7148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc03401a9aeb966f33298268e53ab1040d007224
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-HasComments: Yes