Discussion:
Bug#853568: [Debian-med-packaging] Bug#853568: No idea how to fix abs arguments in nanopolish
(too old to reply)
Gert Wollny
2017-09-01 08:00:03 UTC
Permalink
Hi,
to fix bug #853568 I tried a patch (gcc-7.patch) to fix abs()
arguments
...
g++ -o src/hmm/nanopolish_pore_model_set.o -c -g -O2 -fdebug-
prefix-map=/build/nanopolish-0.5.0=. -fstack-protector-strong
-Wformat -Werror=format-security -g -O3 -std=c++11 -fopenmp -Wdate-
t
src/common/nanopolish_variant.cpp: In function
'std::vector<Variant> extract_variants(const string&, const
src/common/nanopolish_variant.cpp:32:69: error: call of overloaded
'abs(std::__cxx11::basic_string<char>::size_type)' is ambiguous
     size_t difference = std::abs(reference.size() -
haplotype.size());
The result of subtracting two size_t's is still a size_t, which is
unsigned.  So you need to cast it to a signed type.  The correct type
is ptrdiff_t.
  http://en.cppreference.com/w/cpp/types/ptrdiff_t
The line then becomes
  size_t difference =
std::abs(static_cast<ptrdiff_t>(reference.size() -
haplotype.size()));
Casting the difference may result in undefined behavior. Consider the
case

reference.size() == 1
haplotype.size() == 2

then 

reference.size() - haplotype.size() 

will be 0xFFFFFFFF (on 32 bit), and how this is casted to a signed type
is implementation dependent (i.e. it is not guaranteed that this simply
wraps to -1, it may also raise a trap because of integer overflow).

It would be better to avoid the cast altogether by doing something like

  size_t difference = reference.size() > haplotype.size() ?
reference.size() - haplotype.size() :
haplotype.size() - reference.size();


or cast both values before doing the subtraction.

Best,
Gert
Christian Seiler
2017-09-18 16:40:01 UTC
Permalink
Hi Andreas,
Strangely enough on i386 the build fails with
/usr/bin/ld: cannot find -lhdf5
which I do not understand as well ...
You add the following to the linker flags:

-L/usr/lib/$(shell dpkg-architecture -qDEB_TARGET_GNU_TYPE)/hdf5/serial

This is wrong on i386: DEB_TARGET_GNU_TYPE expands to i686-linux-gnu,
while Debian uses i386-linux-gnu. Also, DEB_TARGET_* is definitely
wrong unless you are _building_ a cross-compiler. What you want here is
DEB_HOST_MULTIARCH - that will be correct even if you are _using_ a
cross compiler.

Also, if the package requires intrinsics, you should depend on
sse-support on i386 (but not on amd64, where SSE1 is always part of
the base ISA).

Regards,
Christian

Loading...