[ 
https://issues.apache.org/jira/browse/IMPALA-9762?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17120055#comment-17120055
 ] 

ASF subversion and git services commented on IMPALA-9762:
---------------------------------------------------------

Commit f474b03dce35956d0762159eed16516b793903eb in impala's branch 
refs/heads/master from Joe McDonnell
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=f474b03 ]

IMPALA-9762: Fix GCC7 shift-count-overflow in tuple-row-compare.cc

This fixes a GCC 7 compilation error for this code in
TupleRowZOrderComparator's GetSharedIntRepresentation() and
GetSharedFloatRepresentation():

  return (static_cast<U>(val) <<
      std::max((sizeof(U) - sizeof(T)) * 8, (uint64_t)0)) ^ mask;

In this case, the std::max is running with uint64_t arguments. For
template instatiations with sizeof(T) > sizeof(U), this results
in integer overflow and a very large positive integer causing
the shift-count-overflow. These instantiations are not used by
Impala, but the compiler still needs to generate them.

This changes the logic to use signed integers for the std::max,
avoiding the shift-count-overflow.

Testing:
 - Build on GCC 4.9.2 and GCC 7
 - Core tests

Change-Id: I518e8bed1bb8d49d9cb76a33b07b665e15dfef87
Reviewed-on: http://gerrit.cloudera.org:8080/15962
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>


> Fix GCC7 compilation issue: shift-count-overflow in tuple-row-compare.cc
> ------------------------------------------------------------------------
>
>                 Key: IMPALA-9762
>                 URL: https://issues.apache.org/jira/browse/IMPALA-9762
>             Project: IMPALA
>          Issue Type: Bug
>          Components: Backend
>    Affects Versions: Impala 4.0
>            Reporter: Joe McDonnell
>            Priority: Major
>
> GCC 7 compilation fails on tuple-row-compare.cc with the following:
>  
> {noformat}
> /home/joe/view2/Impala/be/src/util/tuple-row-compare.cc: In instantiation of 
> \u2018U impala::TupleRowZOrderComparator::GetSharedFloatRepresentation(void*, 
> U) const [with U = unsigned int; T = double]\u2019:
> /home/joe/view2/Impala/be/src/util/tuple-row-compare.cc:405:53:   required 
> from \u2018U impala::TupleRowZOrderComparator::GetSharedRepresentation(void*, 
> impala::ColumnType) const [with U = unsigned int]\u2019
> /home/joe/view2/Impala/be/src/util/tuple-row-compare.cc:357:41:   required 
> from \u2018int impala::TupleRowZOrderComparator::CompareBasedOnSize(const 
> impala::TupleRow*, const impala::TupleRow*) const [with U = unsigned 
> int]\u2019
> /home/joe/view2/Impala/be/src/util/tuple-row-compare.cc:343:49:   required 
> from here
> /home/joe/view2/Impala/be/src/util/tuple-row-compare.cc:454:33: error: left 
> shift count >= width of type [-Werror=shift-count-overflow]
>      return static_cast<U>(~tmp) << std::max((sizeof(U) - sizeof(T)) * 8, 
> (uint64_t)0);
>             
> ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/joe/view2/Impala/be/src/util/tuple-row-compare.cc:457:33: error: left 
> shift count >= width of type [-Werror=shift-count-overflow]
>      return (static_cast<U>(tmp) << std::max((sizeof(U) - sizeof(T)) * 8, 
> (uint64_t)0)) ^
>             
> ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1plus: all warnings being treated as errors
> {noformat}
>  
> What is happening is that GCC is creating some template instantiations of 
> TupleRowZOrderComparator::GetSharedRepresentation() and 
> TupleRowZOrderComparator::CompareBasedOnSize(). The code at issue is doing 
> this calculation:
>  
> {code:java}
>   return (static_cast<U>(val) <<
>       std::max((sizeof(U) - sizeof(T)) * 8, (uint64_t)0)) ^ mask;
> {code}
>  
> However, for these template instantiations, sizeof(T) > sizeof(U). That 
> condition is impossible and never executed in Impala, but the instantiation 
> exists. The compiler is intepreting the sizeof(U) - sizeof(T) not as a 
> negative integer, but as a very very large positive integer (maybe due to the 
> uint64_t 0). It wins the max calculation and triggers the 
> shift-count-overflow.
> The fix is to do the max calculation with signed integers and then cast it to 
> unsigned for the shift.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to