Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/12280 )
Change subject: IMPALA-5031: out-of-range enum values are undefined behavior ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12280/1/be/src/exec/parquet/parquet-metadata-utils.cc File be/src/exec/parquet/parquet-metadata-utils.cc: http://gerrit.cloudera.org:8080/#/c/12280/1/be/src/exec/parquet/parquet-metadata-utils.cc@216 PS1, Line 216: const auto codec = Ubsan::EnumToInt(&col_chunk_metadata.codec); > Wouldn't we have already caused some kind of undefined behaviour earlier wh It depends on the mechanism it used, what version of the standard we compile with, and one's interpretation of standard-ese. In particular, static_cast-ing from an out-of-range value to an enum is implementation-defined in C++14 (which, as you know, we are currently using) and undefined in C++17. (However, even in C++17 mode, clang++ 5.0 does not fire a ubsan alert on the static_cast.) I do not think reinterpret_cast-ing produces enum-based UB, but it could produce aliasing UB, depending on how it was originally allocated. Of course, we explicitly allow aliasing with our compiler flags, so we intend to force that kind of UB into defined behavior. -- To view, visit http://gerrit.cloudera.org:8080/12280 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iecdf301065da8f091a274e7a0585a11fcc79da7d Gerrit-Change-Number: 12280 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Mon, 28 Jan 2019 18:05:22 +0000 Gerrit-HasComments: Yes
