Build failure/failure with `-Wconversion`

[LeSuisse]

Hello,

A build failure/warning can be encountered when building with -Wconversion.

https://github.com/trailofbits/pe-parse/blob/8d8daa5ccc3cae1a67a263d07e550bb05bae9e57/pe-parser-library/src/parse.cpp#L1780

The issue has been identified via the NixOS/nixpkgs project:

$ nix-build -A pe-parse
this derivation will be built:
  /nix/store/ljapc56d29pwl62khnvq9961wvmvjgir-pe-parse-2.0.0.drv
building '/nix/store/ljapc56d29pwl62khnvq9961wvmvjgir-pe-parse-2.0.0.drv'...
unpacking sources
unpacking source archive /nix/store/wclsd3p81ywpj12gs16x50ilz8wc91f6-source
source root is source
patching sources
configuring
fixing cmake files...
cmake flags: -DCMAKE_FIND_USE_SYSTEM_PACKAGE_REGISTRY=OFF -DCMAKE_FIND_USE_PACKAGE_REGISTRY=OFF -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_SKIP_BUILD_RPATH=ON -DBUILD_TESTING=OFF -DCMAKE_INSTALL_LOCALEDIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/share/locale -DCMAKE_INSTALL_LIBEXECDIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/libexec -DCMAKE_INSTALL_LIBDIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/lib -DCMAKE_INSTALL_DOCDIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/share/doc/pe-parse -DCMAKE_INSTALL_INFODIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/share/info -DCMAKE_INSTALL_MANDIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/share/man -DCMAKE_INSTALL_OLDINCLUDEDIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/include -DCMAKE_INSTALL_INCLUDEDIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/include -DCMAKE_INSTALL_SBINDIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/sbin -DCMAKE_INSTALL_BINDIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/bin -DCMAKE_INSTALL_NAME_DIR=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0/lib -DCMAKE_POLICY_DEFAULT_CMP0025=NEW -DCMAKE_OSX_SYSROOT= -DCMAKE_FIND_FRAMEWORK=LAST -DCMAKE_STRIP=/nix/store/bg35nfwn6zd616facdywiysgpprfvsji-gcc-wrapper-11.3.0/bin/strip -DCMAKE_RANLIB=/nix/store/rs684lgm8k7akkgbisb49z4vpxxc2zns-binutils-2.38/bin/ranlib -DCMAKE_AR=/nix/store/rs684lgm8k7akkgbisb49z4vpxxc2zns-binutils-2.38/bin/ar -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_INSTALL_PREFIX=/nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0  
-- The C compiler identification is GNU 11.3.0
-- The CXX compiler identification is GNU 11.3.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /nix/store/bg35nfwn6zd616facdywiysgpprfvsji-gcc-wrapper-11.3.0/bin/gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /nix/store/bg35nfwn6zd616facdywiysgpprfvsji-gcc-wrapper-11.3.0/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Looking for C++ include filesystem
-- Looking for C++ include filesystem - found
-- Performing Test CXX_FILESYSTEM_NO_LINK_NEEDED
-- Performing Test CXX_FILESYSTEM_NO_LINK_NEEDED - Success
-- VERSION file: /build/source/pe-parser-library/../VERSION
-- Build type: Release
-- Build Shared: ON 
-- Build Command Line Tools: ON
-- Install prefix: /nix/store/6imj7f4cpj59mjhnhi7azbc0b3jbn0bv-pe-parse-2.0.0
-- Configuring done
-- Generating done
CMake Warning:
  Manually-specified variables were not used by the project:

    BUILD_TESTING
    CMAKE_EXPORT_NO_PACKAGE_REGISTRY
    CMAKE_INSTALL_BINDIR
    CMAKE_INSTALL_DOCDIR
    CMAKE_INSTALL_INCLUDEDIR
    CMAKE_INSTALL_INFODIR
    CMAKE_INSTALL_LIBDIR
    CMAKE_INSTALL_LIBEXECDIR
    CMAKE_INSTALL_LOCALEDIR
    CMAKE_INSTALL_MANDIR
    CMAKE_INSTALL_OLDINCLUDEDIR
    CMAKE_INSTALL_SBINDIR
    CMAKE_POLICY_DEFAULT_CMP0025

-- Build files have been written to: /build/source/build
cmake: enabled parallel building
building
build flags: -j16 -l16 SHELL=/nix/store/07ln9bxp9k8nds669r24fsywf4d1jlly-bash-5.1-p16/bin/bash
[ 16%] Building CXX object pe-parser-library/CMakeFiles/pe-parse.dir/src/buffer.cpp.o
[ 33%] Building CXX object pe-parser-library/CMakeFiles/pe-parse.dir/src/unicode_codecvt.cpp.o
[ 50%] Building CXX object pe-parser-library/CMakeFiles/pe-parse.dir/src/parse.cpp.o
/build/source/pe-parser-library/src/parse.cpp: In function 'bool peparse::getRelocations(peparse::parsed_pe*)':
/build/source/pe-parser-library/src/parse.cpp:1780:24: error: unsigned conversion from 'int' to 'uint16_t' {aka 'short unsigned int'} changes the value of '-61441' [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wsign-conversion-Werror=sign-conversion8;;]
 1780 |         offset = entry & ~0xf000;
      |                  ~~~~~~^~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [pe-parser-library/CMakeFiles/pe-parse.dir/build.make:90: pe-parser-library/CMakeFiles/pe-parse.dir/src/parse.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:143: pe-parser-library/CMakeFiles/pe-parse.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
error: builder for '/nix/store/ljapc56d29pwl62khnvq9961wvmvjgir-pe-parse-2.0.0.drv' failed with exit code 2;
       last 10 log lines:
       > [ 33%] Building CXX object pe-parser-library/CMakeFiles/pe-parse.dir/src/unicode_codecvt.cpp.o
       > [ 50%] Building CXX object pe-parser-library/CMakeFiles/pe-parse.dir/src/parse.cpp.o
       > /build/source/pe-parser-library/src/parse.cpp: In function 'bool peparse::getRelocations(peparse::parsed_pe*)':
       > /build/source/pe-parser-library/src/parse.cpp:1780:24: error: unsigned conversion from 'int' to 'uint16_t' {aka 'short unsigned int'} changes the value of '-61441' [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wsign-conversion-Werror=sign-conversion8;;]
       >  1780 |         offset = entry & ~0xf000;
       >       |                  ~~~~~~^~~~~~~~~
       > cc1plus: all warnings being treated as errors
       > make[2]: *** [pe-parser-library/CMakeFiles/pe-parse.dir/build.make:90: pe-parser-library/CMakeFiles/pe-parse.dir/src/parse.cpp.o] Error 1
       > make[1]: *** [CMakeFiles/Makefile2:143: pe-parser-library/CMakeFiles/pe-parse.dir/all] Error 2
       > make: *** [Makefile:136: all] Error 2
       For full logs, run 'nix log /nix/store/ljapc56d29pwl62khnvq9961wvmvjgir-pe-parse-2.0.0.drv'.

[woodruffw]

Thanks for the report @LeSuisse. I'll see about a fix today.

[woodruffw]

@LeSuisse to clarify: how many errors are you seeing with -Wconversion? I'd like to enable it globally for our own builds, but we do quite a bit of sizeof/offsetof arithmetic internally that'll need to be explicitly cast/refactored. If that site is the only one that's currently causing problems for Nix builds, I can do a smaller changeset fixing it first.

[LeSuisse]

I only saw those 2 issues (it is the full build log in first post) but I did not investigate further. A local fix might enough :smile:

[woodruffw]

I think #170 should fix the specific warnings here. I can cut a release, if that'll make things easier on your end.

[LeSuisse]

I just tested with the patch and it looks good (https://github.com/NixOS/nixpkgs/pull/173037), thanks!

Cutting a release is not necessary for nixpkgs, we can pull the patch directly so there is no rush.

[woodruffw]

Excellent, good to know. We’ll probably do a release in a couple of weeks, then. Thanks again for reporting.

Sent from mobile. Please excuse my brevity.

On May 14, 2022, at 11:22 AM, Thomas Gerbet @.***> wrote:

 Closed #169.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were assigned.