Build fails with LTO

[eli-schwartz]

I tried to build abyss-2.3.4 with the following *FLAGS to optimize the build: -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing

Note the -Werror=* flags are used to help detect cases where the compiler tries to optimize by assuming UB cannot exist in the source code -- if it does exist, ordinarily the code would be miscompiled, and this says to make the miscompilation a fatal error.

I got this build error:

x86_64-pc-linux-gnu-gcc  -march=native -fstack-protector-all -O2 -pipe -fdiagnostics-color=always -frecord-gcc-switches -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-clash-protection -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing  -Wformat -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=int-conversion -Werror=incompatible-pointer-types  -Wl,-O1 -Wl,--as-needed -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Wl,--defsym=__gentoo_check_ldflags__=0 -o dialign museq.o libdialign.a -ldl -lm 
museq.c:58:13: error: type of ‘simple_aligner’ does not match original declaration [-Werror=lto-type-mismatch]
   58 | extern int  simple_aligner(struct seq_col *scol, struct diag_col *dcol,
      |             ^
assemble.c:398:6: note: return value type mismatch
  398 | char simple_aligner(struct seq_col *scol, struct diag_col *dcol,
      |      ^
assemble.c:398:6: note: type ‘char’ should match type ‘int’
assemble.c:398:6: note: ‘simple_aligner’ was previously declared here
lto1: some warnings being treated as errors
lto-wrapper: fatal error: x86_64-pc-linux-gnu-gcc returned 1 exit status
compilation terminated.
/usr/lib/gcc/x86_64-pc-linux-gnu/13/../../../../x86_64-pc-linux-gnu/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status

Originally reported downstream: https://bugs.gentoo.org/862252 Full build log: build.log

[lcoombe]

@parham-k / @jwcodee - Could one of you please take a look at this?

[lcoombe]

Pinging @parham-k / @jwcodee

[jwcodee]

Thanks for flagging this issue. We will be looking to implement a fix to address this issue.

[jwcodee]

@eli-schwartz can you see if https://github.com/bcgsc/abyss/tree/tlo addresses your issue?

[eli-schwartz]

There are 3 definitions:

• dialign/museq.c: extern int simple_aligner
• dialign/assemble.c: char simple_aligner
• Align/dialign.h: char simple_aligner

It looks like your tree changes the second one from char to int, which means now the third is the odd one out.

[jwcodee]

I noticed that too, but I didn't see assemble.c including the dialign.h header so I didn't change the char there.

What parts of the build script did you add the following flags: -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing?

[eli-schwartz]

I noticed that too, but I didn't see assemble.c including the dialign.h header so I didn't change the char there.

I don't think it has to? assemble.c has the (only) function implementation and that appears early on in the file before any actual uses.

What parts of the build script did you add the following flags: -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing?

It was exported before running configure, so it applies to all sources.

[eli-schwartz]

Keep in mind that what LTO is doing here is simply being a massively global optimization pass that is able to keep track of the entire linked program, including full type info, all at the same time. This means that it can double check that the symbol from assemble.c is compatible with the prototype in dialign.h and the additional prototype in museq.c

Since they are all referring to the same outputted machine code symbol anyway, they have to be compatible or the compiler might generate faulty code. Often, this passes silently if prototypes are incorrect. Due to the greatly expanded tracking capabilities afforded by the LTO optimizer, it is simply able to provide better diagnostics and uncover otherwise very hard to find issues.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your interest in ABySS!

[lcoombe]

Any updates on this, @jwcodee ?

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your interest in ABySS!

[lcoombe]

Any updates on this, @jwcodee?

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your interest in ABySS!

[eli-schwartz]

If you take a look at the edit history for the first comment, you'll see something interesting. Now that I no longer have to live in fear that any comment will inadvertently trick the stale-bot into mistakenly believing I am engaging in "further activity" (I have no further activity to engage in, since I have given all the information I believe is relevant and haven't been asked any new questions), I figure this is a good time to post it as a new comment of its own.

---

EDIT: May 2

github-actions bot commented 16 minutes ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your interest in ABySS!

I do not fight with stale bots. I don't spend my time "having recent activity" to convince a hostile workflow that an acknowledged issue is important enough to keep open.

---

Parting notes:

https://nostalebots.xyz/ https://fvsch.com/stale-bots https://drewdevault.com/2021/10/26/stalebot.html https://blog.benwinding.com/github-stale-bots/

[lcoombe]

I'm sorry you feel that way, @eli-schwartz. As a rather small academic group, we have limited resources and do our best to help users out, but it can be difficult as we do not have a dedicated developer for this project.

@jwcodee Could you please comment on any updates on your end?

[jwcodee]

Sorry for the delay. The change is made and is passing CI and we will be pushing @eli-schwartz your suggestion (https://github.com/bcgsc/abyss/tree/tlo ) to master once we have checked the assembly results internally and ensure that no unintended consequences are made.

-ffat-lto-objects was necessary in addition to your flags to compile ABySS with LTO

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your interest in ABySS!

[jwcodee]

Processing CI issues related to the PR

[jwcodee]

Done. See #479