#10259 closed defect (wontfix)

gcc 4.8 with CFLAGS=-Os generates infinite loop in smartlist_ensure_capacity()

Reported by: zougloub Owned by: nick
Priority: normal Milestone: Tor: 0.2.5.x-final
Component: Tor Version: Tor: 0.2.5.1-alpha
Keywords: compiler, tor-client, 024-backport, 025-triaged andrea-review-0255 Cc:
Actual Points: Parent ID:
Points:

Description

Tested on x86_64 with gcc 4.8.2.

CFLAGS="-march=native -Os"

gcc generates the smartlist_ensure_capacity() function code:

Dump of assembler code for function smartlist_ensure_capacity:
   0x00000000004c2ad1 <+0>:     push   %rbx
   0x00000000004c2ad2 <+1>:     mov    %rdi,%rbx
   0x00000000004c2ad5 <+4>:     cmp    0xc(%rdi),%esi
   0x00000000004c2ad8 <+7>:     jle    0x4c2b00 <smartlist_ensure_capacity+47>
   0x00000000004c2ada <+9>:     cmp    $0x3fffffff,%esi
   0x00000000004c2ae0 <+15>:    jg     0x4c2ae4 <smartlist_ensure_capacity+19>
   0x00000000004c2ae2 <+17>:    jmp    0x4c2ae2 <smartlist_ensure_capacity+17>
   0x00000000004c2ae4 <+19>:    movl   $0x7fffffff,0xc(%rdi)
   0x00000000004c2aeb <+26>:    mov    (%rdi),%rdi
   0x00000000004c2aee <+29>:    movabs $0x3fffffff8,%rsi
   0x00000000004c2af8 <+39>:    callq  0x4ca268 <tor_realloc_>
   0x00000000004c2afd <+44>:    mov    %rax,(%rbx)
   0x00000000004c2b00 <+47>:    pop    %rbx
   0x00000000004c2b01 <+48>:    retq   
End of assembler dump.

We can clearly see that 0x00000000004c2ae2 <+17>: jmp 0x4c2ae2 <smartlist_ensure_capacity+17> is an infinite loop.

If now, we *use* the size parameter, for instance by doing the following at the beginning of the function:

  sl->trick = size;

Then gcc generates correct code:

Dump of assembler code for function smartlist_ensure_capacity:
   0x00000000004c2ad1 <+0>:     push   %rbx
   0x00000000004c2ad2 <+1>:     mov    %rdi,%rbx
   0x00000000004c2ad5 <+4>:     mov    0xc(%rdi),%eax
   0x00000000004c2ad8 <+7>:     mov    %esi,0x10(%rdi)
   0x00000000004c2adb <+10>:    cmp    %eax,%esi
   0x00000000004c2add <+12>:    jle    0x4c2b0c <smartlist_ensure_capacity+59>
   0x00000000004c2adf <+14>:    cmp    $0x3fffffff,%esi
   0x00000000004c2ae5 <+20>:    jg     0x4c2aef <smartlist_ensure_capacity+30>
   0x00000000004c2ae7 <+22>:    add    %eax,%eax
   0x00000000004c2ae9 <+24>:    cmp    %eax,%esi
   0x00000000004c2aeb <+26>:    jg     0x4c2ae7 <smartlist_ensure_capacity+22>
   0x00000000004c2aed <+28>:    jmp    0x4c2af4 <smartlist_ensure_capacity+35>
   0x00000000004c2aef <+30>:    mov    $0x7fffffff,%eax
   0x00000000004c2af4 <+35>:    mov    (%rbx),%rdi
   0x00000000004c2af7 <+38>:    mov    %eax,0xc(%rbx)
   0x00000000004c2afa <+41>:    cltq   
   0x00000000004c2afc <+43>:    lea    0x0(,%rax,8),%rsi
   0x00000000004c2b04 <+51>:    callq  0x4ca274 <tor_realloc_>
   0x00000000004c2b09 <+56>:    mov    %rax,(%rbx)
   0x00000000004c2b0c <+59>:    pop    %rbx
   0x00000000004c2b0d <+60>:    retq   
End of assembler dump.

Here, no infinite loop anymore.

As of 2013-11-30T23:19:42 EST I'm trying to dig further.

Child Tickets

Change History (18)

comment:1 Changed 21 months ago by zougloub

  • Summary changed from smartlist_ensure_capacity() issue with gcc 4.8 to gcc 4.8 with CFLAGS=-Os generates infinite loop in smartlist_ensure_capacity()

comment:2 Changed 21 months ago by zougloub

Turns out that:

  • CFLAGS="-O2" or "-O3" generates correct code
  • CFLAGS="-Os" generates the infinite loop

Will check what -Os does more.

comment:3 Changed 21 months ago by zougloub

Checked what -Os does compared to -O2:

gcc -O2 -Q --help=optimizers > a
gcc -Os -Q --help=optimizers > b
diff -U0 a b

This gives:

--- a
+++ b
-  -finline-functions                   [disabled]
+  -finline-functions                   [enabled]
-  -foptimize-strlen                    [enabled]
+  -foptimize-strlen                    [disabled]

Blind tested the compilation flags:

tests=(
 "-O2"
 "-O2 -finline-functions"
 "-O2 -fno-optimize-strlen"
 "-O2 -finline-functions -fno-optimize-strlen"
 "-Os"
 "-Os -fno-inline-functions"
 "-Os -foptimize-strlen"
 "-Os -fno-inline-functions -foptimize-strlen"
)

for x in "${tests[@]}";
do
  echo -n "Compilation with ${x}..."
  ./configure CFLAGS="$x" >/dev/null
  make clean >/dev/null
  make >/dev/null 2>&1
  cp src/or/tor "tor $x"
  echo "OK"
done

for x in "${tests[@]}";
do
  echo "Starting check of $x"
  gdb "tor $x" -ex "disas smartlist_ensure_capacity" -ex q \
   | perl -ne 'print if /<\+(\d+)>.*jmp.*smartlist_ensure_capacity\+\1/'
  ./"tor $x" -f ~/.tor/rc --runasdaemon 0 &
  sleep 3
  kill %1
  echo "Finished check of $x"
done

Results: the above lags after -Os do not matter, there's always an infinite loop when -Os is involved here.
There's something more than these 2 flags... "gcc -Q --help=optimizers" is not helping much.

comment:4 Changed 21 months ago by zougloub

Got help from o11c and pinskia from #gcc at OFTC.
They reduced the code into a minimal shell that reproduces the problem: https://gist.github.com/o11c/7729355

comment:5 Changed 21 months ago by zougloub

Was suggested to file a gcc bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59358

comment:6 Changed 21 months ago by nickm

Okay; it sounds like the answer here is to get the issue fixed in GCC, assuming that there's nothing wrong with the Tor code. Are there any operating systems which build Tor with -Os by default? If so we should warn their maintainers about the issue.

comment:7 Changed 21 months ago by cypherpunks

It happens for -O2 too, actually. But seems like "fixed" for Tor by -fwrapv. Tested with Interactive compiler.

comment:8 Changed 21 months ago by zougloub

The root cause has been addressed by the gcc developers.

I'll contact the Gentoo packagers to warn about that, as the CFLAGS are user-controlled there.

Recommend closing as invalid.

comment:9 Changed 21 months ago by arma

Was this gcc released with this bug?

If so, maybe Tor's build process should detect it and complain?

The bug wasn't in Tor, but assuming somebody else will try using this gcc, closing as invalid will mean confusion in the future.

comment:10 Changed 21 months ago by zougloub

Working around the issue by patching configure.ac to add -fwrapv for gcc 4.8.0-4.8.2 would help, but I can't appreciate how worthwhile it would be. Most distribution packages are compiled with gcc <= 4.7.x.
Searching for "tor gcc 4.8" points to this bug report already.

comment:11 Changed 21 months ago by arma

  • Milestone set to Tor: 0.2.5.x-final

comment:12 Changed 18 months ago by andrea

Triaged: concluded we should still fix for 0.2.5.x because the proposed fix is safe and 4.8.3 is not yet released.

comment:13 Changed 18 months ago by nickm

  • Keywords tor-client 024-backport added
  • Status changed from new to needs_review

See the branch "bug10259_024" in my public repository for a possible workaround patch. It should apply to 0.2.4 or master.

comment:14 Changed 18 months ago by andrea

  • Keywords 025-triaged added

comment:15 Changed 18 months ago by nickm

  • Keywords changed from compiler tor-client 024-backport 025-triaged to compiler, tor-client, 024-backport, 025-triaged

comment:16 Changed 17 months ago by andrea

  • Keywords andrea-review-0255 added

comment:17 Changed 16 months ago by nickm

I am questioning our triage decision. Nobody else has reported this bug: it appears that either the distributions that are shipping GCC 4.8.[012] are correctly backporting the fix for the GCC bug here, or that nobody is using them to build Tor. Perhaps we should close this as wontfix.

comment:18 Changed 16 months ago by nickm

  • Resolution set to wontfix
  • Status changed from needs_review to closed

14:56 < athena> nickm: fine by me re: #10259

Okay. Closing this as "wontfix". Apparently, the GCC packagers of the world are good at backporting fixes. Thank you, GCC packagers!

Note: See TracTickets for help on using tickets.