refactor: Work around gcc compiler bug 90348 #15959

1. 
   MarcoFalke commented at 2:19 pm on May 6, 2019: member

   The gcc compiler creates “optimized” code (-finline-small-functions) that modifies in a second time after it has been initialized with random bits.

   Working around the bug can be achieved in different ways: Moving in into a new const array, rearranging the loop to break the optimization, or moving the array into the outer scope, …

   This fix is needed to release 0.18.1 on 32 bit platforms, thus needs backport.

   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348 Fixes #14580

2. test: Work around gcc compiler bug 90348 faa924a942
3. MarcoFalke added the label Refactoring on May 6, 2019
4. MarcoFalke added the label Tests on May 6, 2019
5. MarcoFalke added the label Needs backport on May 6, 2019
6. MarcoFalke added this to the milestone 0.18.1 on May 6, 2019
7. 
   luke-jr commented at 2:23 pm on May 6, 2019: member

   Wouldn’t it be better to just add -fno-inline-small-functions when a broken compiler version is detected? (Isn’t this bug liable to affect other random bits of code?)
8. 
   laanwj commented at 2:31 pm on May 6, 2019: member

   Wouldn’t it be better to just add -fno-inline-small-functions when a broken compiler version is detected? (Isn’t this bug liable to affect other random bits of code?)

   The one doesn’t rule out the other, though I would personally feel more comfortable with that solution. It’s unclear to me what else this bug might potentially affect.

9. 
   practicalswift commented at 2:54 pm on May 6, 2019: contributor

   Concept ACK on working around the GCC issue but I agree with @luke-jr and @laanwj that introducing -fno-inline-small-functions would be good.

   I think we should add -fno-inline-small-functions when building with GCC and keep the code unchanged: that way we will notice if -fno-inline-small-functions is removed prematurely in the future (that is: before these GCC versions have died out).

10. 
    MarcoFalke commented at 2:58 pm on May 6, 2019: member

    Ok, marking up for grabs for someone to change the build system.
11. MarcoFalke added the label Up for grabs on May 6, 2019
12. MarcoFalke removed the label Needs backport on May 6, 2019
13. MarcoFalke removed this from the milestone 0.18.1 on May 6, 2019
14. 
    gmaxwell commented at 6:54 pm on May 7, 2019: contributor

    I would expect -fno-inline-small-functions to be devastating for performance and I also believe would not completely work around the bug. I believe -fstack-reuse=none would be a more correct, and less impacting, workaround.
15. 
    luke-jr commented at 8:42 pm on May 7, 2019: member

    I wonder if there’s a reliable compile test we could have configure run to detect the actual bug?
16. 
    MarcoFalke commented at 8:44 pm on May 7, 2019: member

    The test case from the gcc bug wouldn’t work? And if disabling fstack-reuse isn’t too expensive, we might just do it unconditionally?
17. 
    sipa commented at 9:14 pm on May 7, 2019: member

    I would also expect that -fstack-reuse=none has less impact than -fno-inline-small-functions, but benchmarks would be nice.

    I’ll PR the reproduction bug as a unit test.

18. 
    gmaxwell commented at 9:30 pm on May 7, 2019: contributor

    stack-reuse=none also has the advantage of turning some lifetime management bugs into less-bugs (which is AFAICT why the option exists). I would expect it to increase stack usage, which right now we’re not currently particularly careful with.
19. 
    gmaxwell commented at 9:32 pm on May 7, 2019: contributor

    @luke-jr on reliability, I think we’d be better off detecting versions than trying to use the test case. Unfortunately, it’s unclear what versions this will be fixed in: (so far) it appears that this is actually hard for GCC to fix without severely gimping optimization.

    FWIW, the test case does not fail on PPC but I am moderately confident that bug exists there (it might be just that the larger redzone on PPC results in a different stack layout).

20. 
    laanwj commented at 9:51 am on May 8, 2019: member

    Yes, -fstack-reuse=none would be better. Could do it unconditionally for gcc, at least for now. Then when there are versions that patch it, add exceptions.
21. 
    MarcoFalke commented at 6:22 pm on May 8, 2019: member

    Thanks everyone for the input, closing in favour of

    • build with -fstack-reuse=none #15983
22. MarcoFalke closed this on May 8, 2019

    ---

23. MarcoFalke deleted the branch on May 8, 2019
24. MarcoFalke removed the label Up for grabs on May 8, 2019
25. MarcoFalke referenced this in commit 2b56c9a86a on May 16, 2019
26. PastaPastaPasta referenced this in commit e29c493498 on Jun 27, 2021
27. PastaPastaPasta referenced this in commit b2ca1cf545 on Jun 28, 2021
28. PastaPastaPasta referenced this in commit 5125f35a40 on Jun 29, 2021
29. PastaPastaPasta referenced this in commit 661667e794 on Jul 1, 2021
30. PastaPastaPasta referenced this in commit 7fa2a56306 on Jul 1, 2021
31. PastaPastaPasta referenced this in commit 546c385868 on Jul 8, 2021
32. PastaPastaPasta referenced this in commit 7c690ead6b on Jul 10, 2021
33. DrahtBot locked this on Dec 16, 2021

Contributors
MarcoFalke luke-jr laanwj practicalswift gmaxwell sipa

Labels
Refactoring Tests