[Bug target/67716] New: [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[Bug target/67716] New: [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[glaubitz at physik dot fu-berlin.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

            Bug ID: 67716
           Summary: [5] [SH]: Miscompiles libraw: Assembler: unaligned
                    opcodes detected in executable segment
           Product: gcc
           Version: 5.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: glaubitz at physik dot fu-berlin.de
                CC: kkojima at gcc dot gnu.org, olegendo at gcc dot gnu.org
  Target Milestone: ---
            Target: sh*-*-*

Hi!

I have recently triggered rebuilds of all packages with reverse-dependencies on
the new standard C++ library which means all of them were recompiled with gcc-5
after (mostly) being previously built with gcc-4.9.

This resulted in a few FTBFS which previously did not occur, mostly because the
packages need to be ported to the new standard C++ library. However, this
particular package, libraw, seems to fail to build due to a regression, as
gcc-5 generates incorrect assembler code for SH:

libtool: compile:  g++ -DPACKAGE_NAME=\"LibRaw\" -DPACKAGE_TARNAME=\"libraw\"
-DPACKAGE_VERSION=\"0.16.2\" "-DPACKAGE_STRING=\"LibRaw 0.16.2\""
-DPACKAGE_BUGREPORT=\"info@libraw.org\" -DPACKAGE_URL=\"http://www.libraw.org\"
-DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1
-DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1
-DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\"
-DHAVE_JPEGLIB_H=1 -DHAVE_JPEGLIB_H=1 -DHAVE_JASPER_JASPER_H=1 -I.
-D_FORTIFY_SOURCE=2 -I/usr/local/include -DUSE_JPEG -DUSE_JPEG8 -DUSE_JASPER
-DUSE_LCMS2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security
-fopenmp -c internal/dcraw_common.cpp  -fPIC -DPIC -o
internal/.libs/dcraw_common.o
/tmp/ccTfltZR.s: Assembler messages:
/tmp/ccTfltZR.s: Error: unaligned opcodes detected in executable segment
make[1]: *** [internal/dcraw_common.lo] Error 1

I have taken the build directory as is and created a compressed tar ball from
it which can be downloaded here:

https://people.debian.org/~glaubitz/libraw-qDq4D3.tgz

It's just 2.3 MiB in size.

Full build log in [1].

Adrian

> [1] https://buildd.debian.org/status/fetch.php?pkg=libraw&arch=sh4&ver=0.16.2-1%2Bb1&stamp=1443116335

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[kkojima at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #1 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to John Paul Adrian Glaubitz from comment #0)
> I have taken the build directory as is and created a compressed tar ball
> from it which can be downloaded here:

Could you send .i and .s files with adding -save-temps to the problematic
compilation as usual?  The build directory is useful only for the same
debian environment.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[glaubitz at physik dot fu-berlin.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #2 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
(In reply to Kazumoto Kojima from comment #1)
> Could you send .i and .s files with adding -save-temps to the problematic
> compilation as usual?  The build directory is useful only for the same
> debian environment.

Oops, I somehow assumed they were in the tarball but I didn't really check.

Attaching the .i and .s files that were generated with:

glaubitz@tirpitz:~/debian/libraw-test/libraw-qDq4D3/libraw-0.16.2/internal$ gcc
-save-temps -g -O2 -fstack-protector-strong -Wformat -Werror=format-security
-fopenmp -c dcraw_common.cpp  -fPIC -DPIC -o dcraw_common.o -I ..
dcraw_common.s: Assembler messages:
dcraw_common.s: Error: unaligned opcodes detected in executable segment
glaubitz@tirpitz:~/debian/libraw-test/libraw-qDq4D3/libraw-0.16.2/internal$

Interestingly, this problem doesn't occur when omitting all the extra options:

glaubitz@tirpitz:~/debian/libraw-test/libraw-qDq4D3/libraw-0.16.2/internal$ gcc
-c dcraw_common.cpp -o dcraw_common.o -I ..
glaubitz@tirpitz:~/debian/libraw-test/libraw-qDq4D3/libraw-0.16.2/internal$

vs.

glaubitz@tirpitz:~/debian/libraw-test/libraw-qDq4D3/libraw-0.16.2/internal$ gcc
-g -O2 -fstack-protector-strong -Wformat -Werror=format-security -fopenmp -c
dcraw_common.cpp  -fPIC -DPIC -o dcraw_common.o -I ..
/tmp/ccJsoST5.s: Assembler messages:
/tmp/ccJsoST5.s: Error: unaligned opcodes detected in executable segment
glaubitz@tirpitz:~/debian/libraw-test/libraw-qDq4D3/libraw-0.16.2/internal$

I assume, it's one of -f options, but I didn't check yet.

Adrian

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[glaubitz at physik dot fu-berlin.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #3 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
Created attachment 36388
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36388&action=edit
Compressed C++ source for dcraw_common.cpp

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[glaubitz at physik dot fu-berlin.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #4 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
Created attachment 36389
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36389&action=edit
Compressed, preprocessed C++ source for dcraw_common.cpp

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[glaubitz at physik dot fu-berlin.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #5 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
Created attachment 36390
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36390&action=edit
Compressed, assembled C++ source for dcraw_common.cpp

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[glaubitz at physik dot fu-berlin.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #6 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
Attachment 36390 is not assembled, of course. But the actual assembler output
from gcc :).

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #7 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Created attachment 36394
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36394&action=edit
preprocessed C++ source for dcraw_common.cpp

The code in attachment 36389 doesn't compile with the trunk compiler because of
some integer literal / type mismatches.  Adding some casts in the tables fixes
the issue.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #8 from Oleg Endo <olegendo at gcc dot gnu.org> ---
On sh-elf/newlib there are no threads, so -fopenmp doesn't work.  I can't
reproduce it Without -fopenmp.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[kkojima at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #9 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Created attachment 36395
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36395&action=edit
reduced test case

This one fails with the same asm error with my sh-elf c++ for -g -O1.
It looks that
#pragma GCC optimize("no-aggressive-loop-optimizations")
causes unaligned insn after jump table data:

        .align 2
.L4:
        .byte   .L3-.L5
        .byte   .L2-.L5
        .byte   .L2-.L5
        .byte   .L2-.L5
        .byte   .L6-.L5
        .byte   .L2-.L5
        .byte   .L6-.L5
        .byte   .L2-.L5
        .byte   .L2-.L5
        .byte   .L2-.L5
        .byte   .L7-.L5
.L3:
        mov.l   .L16,r1    <<<- not aligned!

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-09-26
     Ever confirmed|0                           |1

--- Comment #10 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #9)
> Created attachment 36395 [details]
> reduced test case
> 
> This one fails with the same asm error with my sh-elf c++ for -g -O1.

Yep, I get this, too.  There should be an ".align 1" after the table.  Or maybe
".align 2" is even safer?


It's strange ... when compiling with -O1 but without -g I get this:

        .long   ___divdi3
        .align 2
.L4:
        .byte   .L3-.L5      // aligned
        .byte   .L2-.L5      // unaligned
        .byte   .L2-.L5      // a
        .byte   .L2-.L5      // u
        .byte   .L6-.L5      // a
        .byte   .L2-.L5      // u
        .byte   .L6-.L5      // a
        .byte   .L2-.L5      // u
        .byte   .L2-.L5      // a
        .byte   .L2-.L5      // u
        .byte   .L7-.L5      // a
.L3:
        mov.l   .L16,r1      // u

but AS does not output an error message.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[kkojima at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #11 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Created attachment 36397
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36397&action=edit
patch for targetm.override_options_after_change

Could you try this patch?

What is going on:

1. align_jumps (global_options.x_align_jumps) is set to 1 as the default value.
2. sh_override_option changes it to 2.
3. pragma handling saves it and sets it back to the default value 1.
4. compile function where align_jumps is 1.
   This results a bad code with wrong alignments.
5. Restore align_jumps at the end of compilation of this unit.

I've noticed that other targets define TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE
and avoid this issue of pragma for optimization.  See aarch64 or alpha's
*_override_options_after_change for example.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #12 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #11)
> Created attachment 36397 [details]
> patch for targetm.override_options_after_change
> 
> Could you try this patch?
> 
> What is going on:
> 
> 1. align_jumps (global_options.x_align_jumps) is set to 1 as the default
> value.
> 2. sh_override_option changes it to 2.
> 3. pragma handling saves it and sets it back to the default value 1.
> 4. compile function where align_jumps is 1.
>    This results a bad code with wrong alignments.
> 5. Restore align_jumps at the end of compilation of this unit.
> 
> I've noticed that other targets define TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE
> and avoid this issue of pragma for optimization.  See aarch64 or alpha's
> *_override_options_after_change for example.

Ah right.  I missed the #pragma thing in your reduced case, sorry.  With your
patch, the .align 1 is back after the table.  Maybe we should move some more of
the sh_option_override things sh_override_options_after_change?  I don't know
...

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[kkojima at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #13 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #12)
> Maybe we should move some
> more of the sh_option_override things sh_override_options_after_change?  I
> don't know ...

I thought the same thing too.  From the implementations for other targets,
the align related flags are enough, I guess.  I could be wrong, though.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #14 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #13)
> (In reply to Oleg Endo from comment #12)
> > Maybe we should move some
> > more of the sh_option_override things sh_override_options_after_change?  I
> > don't know ...
> 
> I thought the same thing too.  From the implementations for other targets,
> the align related flags are enough, I guess.  I could be wrong, though.

Yes, there are issues.  I've created PR 67723.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[kkojima at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #15 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #14)
> Yes, there are issues.  I've created PR 67723.

Ah, you are right.  I forgot -m optimization options at all.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #16 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #15)
> (In reply to Oleg Endo from comment #14)
> > Yes, there are issues.  I've created PR 67723.
> 
> Ah, you are right.  I forgot -m optimization options at all.

Regardless of those, Kaz, can you please commit attachment 36397?  Then I can
handle the other cases on top of that.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[kkojima at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #17 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Author: kkojima
Date: Tue Sep 29 05:36:01 2015
New Revision: 228228

URL: https://gcc.gnu.org/viewcvs?rev=228228&root=gcc&view=rev
Log:
PR target/67716
* [SH] Implement targetm.override_options_after_change hook
  so to avoid resetting loop, jump and function alignment values with
  function-wise optimization flags.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.c

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[kkojima at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #18 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #16)
> Regardless of those, Kaz, can you please commit attachment 36397 [details]? 
> Then I can handle the other cases on top of that.

Done.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #19 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #18)
> (In reply to Oleg Endo from comment #16)
> > Regardless of those, Kaz, can you please commit attachment 36397 [details]? 
> > Then I can handle the other cases on top of that.
> 
> Done.

Thank you.  Since this was reported for GCC 5, I think it should be backported.
 It's not a regression, but a latent issue.  If you have no objections, I can
do it.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[kkojima at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #20 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #19)

I have no objection.  I thought that the fix for PR67723 on trunk is first,
though either way will be fine.

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

--- Comment #21 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Sat Oct  3 15:20:58 2015
New Revision: 228449

URL: https://gcc.gnu.org/viewcvs?rev=228449&root=gcc&view=rev
Log:
gcc/
        Backport from mainline
        2015-09-29  Kaz Kojima  <kkojima@gcc.gnu.org>

        PR target/67716
        * config/sh/sh.c (sh_override_options_after_change): New.
        (TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE): Define.
        (sh_option_override): Move align_loops, align_jumps and
        align_functions handling into sh_override_options_after_change.

Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/config/sh/sh.c

[Bug target/67716] [5] [SH]: Miscompiles libraw: Assembler: unaligned opcodes detected in executable segment

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67716

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #22 from Oleg Endo <olegendo at gcc dot gnu.org> ---
I think we can close this as fixed.