[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 #23 from John Paul Adrian Glaubitz  ---
(In reply to Oleg Endo from comment #22)
> I think we can close this as fixed.

Yes, I can confirm libraw now builds fine. Full build log available at [1].

Adrian

> [1] 
> https://buildd.debian.org/status/fetch.php?pkg=libraw=sh4=0.17.0-1=1445047219

[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  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #22 from Oleg Endo  ---
I think we can close this as fixed.

[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  ---
Author: olegendo
Date: Sat Oct  3 15:20:58 2015
New Revision: 228449

URL: https://gcc.gnu.org/viewcvs?rev=228449=gcc=rev
Log:
gcc/
Backport from mainline
2015-09-29  Kaz Kojima  

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

--- Comment #19 from Oleg Endo  ---
(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  ---
(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

[kkojima at gcc dot gnu.org]

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

--- Comment #17 from Kazumoto Kojima  ---
Author: kkojima
Date: Tue Sep 29 05:36:01 2015
New Revision: 228228

URL: https://gcc.gnu.org/viewcvs?rev=228228=gcc=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  ---
(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 #16 from Oleg Endo  ---
(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

[olegendo at gcc dot gnu.org]

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

--- Comment #7 from Oleg Endo  ---
Created attachment 36394
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36394=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

Oleg Endo  changed:

   What|Removed |Added

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

--- Comment #10 from Oleg Endo  ---
(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

[olegendo at gcc dot gnu.org]

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

--- Comment #8 from Oleg Endo  ---
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 #11 from Kazumoto Kojima  ---
Created attachment 36397
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36397=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  ---
(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 #9 from Kazumoto Kojima  ---
Created attachment 36395
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36395=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

[kkojima at gcc dot gnu.org]

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

--- Comment #13 from Kazumoto Kojima  ---
(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

[kkojima at gcc dot gnu.org]

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

--- Comment #15 from Kazumoto Kojima  ---
(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 #14 from Oleg Endo  ---
(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

[glaubitz at physik dot fu-berlin.de]

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

--- Comment #6 from John Paul Adrian Glaubitz  ---
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

[glaubitz at physik dot fu-berlin.de]

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

--- Comment #3 from John Paul Adrian Glaubitz  ---
Created attachment 36388
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36388=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  ---
Created attachment 36389
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36389=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  ---
Created attachment 36390
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36390=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 #2 from John Paul Adrian Glaubitz  ---
(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

[kkojima at gcc dot gnu.org]

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

--- Comment #1 from Kazumoto Kojima  ---
(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.