On 5 Nov 2024, at 21:27, Aurelien Jarno <aure...@debian.org> wrote: > > Source: lziprecover > Version: 1.25~pre1-1 > Severity: serious > Tags: ftbfs upstream > Justification: fails to build from source (but built successfully in the past) > X-Debbugs-Cc: debian-ri...@lists.debian.org > User: debian-ri...@lists.debian.org > Usertags: riscv64 > > Dear maintainer, > > lziprecover fails to build on riscv64 with the following error: > > | riscv64-linux-gnu-g++ -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 > -ffile-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat > -Werror=format-security -DPROGVERSION=\"1.25-pre1\" -c -o main.o main.cc > | riscv64-linux-gnu-g++ -g -O2 -ffile-prefix-map=/<<PKGBUILDDIR>>=. > -fstack-protector-strong -Wformat -Werror=format-security -Wl,-z,relro -o > lziprecover arg_parser.o alone_to_lz.o lzip_index.o list.o byte_repair.o > dump_remove.o fec_create.o fec_repair.o gf8.o gf16.o lunzcrash.o md5.o > merge.o mtester.o nrep_stats.o range_dec.o recursive.o reproduce.o split.o > decoder.o main.o -lpthread > | main.o: in function `(anonymous namespace)::set_mode((anonymous > namespace)::Mode&, (anonymous namespace)::Mode) [clone .part.0]': > | /usr/include/riscv64-linux-gnu/bits/stdio2.h:111:(.text+0x8b0): relocation > truncated to fit: R_RISCV_PCREL_HI20 against `.LC15' > | collect2: error: ld returned 1 exit status > | make[1]: *** [Makefile:49: lziprecover] Error 1 > | make[1]: Leaving directory '/<<PKGBUILDDIR>>' > | dh_auto_build: error: make -j4 returned exit code 2 > | make: *** [debian/rules:6: binary-arch] Error 25 > | dpkg-buildpackage: error: debian/rules binary-arch subprocess returned exit > status 2 > | > -------------------------------------------------------------------------------- > > The full build log is available there: > https://buildd.debian.org/status/fetch.php?pkg=lziprecover&arch=riscv64&ver=1.25%7Epre1-1&stamp=1728351091&raw=0 > > After investigation, I have found that this problem occurs when the > compiler inlines the newly added compare_prefix() function. This causes > an out-of-bound access on a constant. This function is, for instance, > called in parse_fec() that way: > > | else if( compare_prefix( arg, "repair" ) ) > | set_mode( program_mode, m_fec_repair ); > > With the default arguments like in the above calls, compare_prefix() can be > simplified as: > > | // return true if arg is a non-empty prefix of target > | bool compare_prefix( const char * const arg, const char * const target) > | { > | if( arg[0] == target[0] ) > | for( int i = 1; i < INT_MAX; ++i ) > | { > | if( arg[i] == 0 ) return true; > | if( arg[i] != target[i] ) break; > | } > | return false; > | } > > As you can see if the length of the arg string is longer than the target > string, the latter is accessed out-of-bounds, possibly up to INT_MAX.
That’s not true. For the next iteration of the loop to occur, arg[i] != 0 and arg[i] == target[i], therefore target[i] != 0 and thus the end of target has not been reached yet. Or, put another way, if we ever reach the end of target, either arg[i] == target[i] == 0 and so we return true, or arg[i] != target[i] == 0 and so we break out of the loop (and return false). This looks completely fine from that perspective to me, with one exception, if arg and target are both the empty string then we walk off the end (there should be a check for arg[0] != 0 in the outer if). But presumably one argument is always a string literal rather than external input so is known to not be empty. Or so we hope. The error above is that there’s a signed 32-bit PC-relative relocation whose value is truncated. That is, the binary is too big for the code model in use. Perhaps what you’re noticing is that previously it was just fine, but inlining has increased the code size and pushed it past the limit. Does the binary embed large arrays of data? Jess > Probably INT_MAX should be replaced by "strlen(target) + 1”.
