Debian Bug report logs - #1086791
gcc: riscv64 backend emits large relocations due to loop strength reduction

Reply or subscribe to this bug.

Display info messages

Message #5 received at [email protected] (full text, mbox, reply):

From: Aurelien Jarno <[email protected]>

To: Debian Bug Tracking System <[email protected]>

Subject: lziprecover: FTBFS on riscv64 due to out-of-bounds access on a constant

Date: Tue, 05 Nov 2024 22:27:00 +0100

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: [email protected]
User: [email protected]
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.
Probably INT_MAX should be replaced by "strlen(target) + 1".

Regards
Aurelien

Message #10 received at [email protected] (full text, mbox, reply):

From: Jessica Clarke <[email protected]>

To: [email protected]

Cc: Aurelien Jarno <[email protected]>

Subject: Re: Bug#1086791: lziprecover: FTBFS on riscv64 due to out-of-bounds access on a constant

Date: Tue, 5 Nov 2024 21:47:24 +0000

On 5 Nov 2024, at 21:27, Aurelien Jarno <[email protected]> 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: [email protected]
> User: [email protected]
> 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”.

Message #15 received at [email protected] (full text, mbox, reply):

From: Aurelien Jarno <[email protected]>

To: Jessica Clarke <[email protected]>

Cc: [email protected], [email protected]

Subject: Re: Bug#1086791: lziprecover: FTBFS on riscv64 due to out-of-bounds access on a constant

Date: Tue, 5 Nov 2024 23:45:20 +0100

control: severity -1 important
control: reassign -1 gcc-14
control: retitle -1 gcc: riscv64 backend emits large relocations due to loop strength reduction
control: forwarded -1 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117460
control: affects -1 lziprecover

On 2024-11-05 21:47, Jessica Clarke wrote:
> On 5 Nov 2024, at 21:27, Aurelien Jarno <[email protected]> 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: [email protected]
> > User: [email protected]
> > 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?

We continued the discussion on IRC, and Jessica pointed me that's
actually a GCC bug that I have now reported upstream. I am therefore
retitling and reassigning this bug accordingly.

Regards
Aurelien 

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
[email protected]                     http://aurel32.net

Send a report that this bug log contains spam.