Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

delimcc/setjmp_amd32 compilation defect #3916

Closed
LeeTibbert opened this issue May 12, 2024 · 6 comments · Fixed by #3926
Closed

delimcc/setjmp_amd32 compilation defect #3916

LeeTibbert opened this issue May 12, 2024 · 6 comments · Fixed by #3926

Comments

@LeeTibbert
Copy link
Contributor

LeeTibbert commented May 12, 2024

When compiling (scala-cli) on linux X86_64 (Intel), I am (since recently?) I am getting:

[error] /usr/bin/ld: warning: /mnt/AthaBHdd1/lee49/DevoHdd1/scala_native/WorkSpace_1/2024/study_jnf_WalkFileTree_NoFSLoopException_I3744_2024-04-30T1053-0400/sandbox/src/main/scala/.scala-build/scala_f187f7cdf9-ed51319312/native/native/dependencies/nativelib_native0.5_3-0.5.1-4/scala-native/delimcc/setjmp_amd32.S.o: missing .note.GNU-stack section implies executable stack
[error] /usr/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker

The long running basic defect is that a 32 bit file is getting compiled at all on a 64 bit machine. (and probably visa versa).

I think the death-by-a-thousand-cuts fix is to change

#endif

#if defined(__linux__) && defined(__ELF__)
/* Reference:                                                                   
 *   https://wiki.gentoo.org/wiki/Hardened/GNU_stack_quickstart                 
 */

.section .note.GNU-stack,"",%progbits

#endif
#endif

to

#endif
#endif


#if defined(__linux__) && defined(__ELF__)
/* Reference:                                                                   
 *   https://wiki.gentoo.org/wiki/Hardened/GNU_stack_quickstart                 
 */

.section .note.GNU-stack,"",%progbits

#endif

That should put the .section note outside of the

#if defined(SCALANATIVE_COMPILE_ALWAYS) || defined(__SCALANATIVE_DELIMCC)
#if defined(__i386__) && (defined(__linux__) || defined(__APPLE__))

guards and ensure that the necessary section note will get generated if the compiler ever touch the the file with
__linux_ && __ELF__.

I have not tried this, I am too deep in javalib Files symlink fun and am still recovering from the trauma from the last
time I touched these files.

I suspect that a similar change needs to be made to all the .S files in that directory.

@WojciechMazur
Copy link
Contributor

Hey, I've tried to reproduce it, but I couldn't receive the error/warning you're getting. I've tried it on GitPod on x86_64, using both latest GNU ld 2.42, and default 2.36. Reproducing it was especially important to me, so we can find out why compilation of this file is triggered at all.

@LeeTibbert
Copy link
Contributor Author

Thank you for taking a look at this. I'll try late tomorrow (Weds, probably Thursday your time) to reproduce.

@LeeTibbert
Copy link
Contributor Author

To get this out of my brain, so that I can deal with the javalib Files "walk*/find" mule which is kicking me,
I am trying to reproduce this in another window.

We are using the same version of ld.

I think the mid to longer range issue is why is the .c (or .cpp) file being presented to the compiler at all?
The compiler is probably creating a zero sized .o file which gets carried along to ld, for it to process.
Overhead all down the line.

I understand why the initial generation went in as it did. Succeeding generations have improved the
short term solution. I have not gotten around to fixing the underlying problem in the build.

@LeeTibbert
Copy link
Contributor Author

TL; DR * I spent several hours and could not reproduce. I will submit a robustness PR anyway.
* I suspect that my clang version changed since I submitted the base report.

Environment: Ubuntu 23.04 Intel X86_64
clang Ubuntu clang version 18.1.3 (1) Target: x86_64-pc-linux-gnu
Scala: 3.3.1
Java: 21

Test command: tests3/testOnly *.ContinuationsTest (passes)

cd ./unit-tests/native/.3/target/scala-3.4.1/native-test/dependencies/nativelib_native0.5_3-0.5.2-SNAPSHOT-1/scala-native/delimcc

ls ls -l *.S.o
-rw------- 1 lee lee  984 May 20 21:33 setjmp_amd32.S.o
-rw------- 1 lee lee 3736 May 20 21:33 setjmp_amd64.S.o
-rw------- 1 lee lee  984 May 20 21:33 setjmp_arm64.S.o
-rw------- 1 lee lee  984 May 20 21:33 setjmp_x64.S.o
 
// Note well: setjmp_amd32.S.o, setjmp_arm64.S.o, & setjmp_x64.S.o are generated & taking up disk space.

// Note: all 4 below now have .note.GNU-stack, so no problem.
//           clang would generate for .c/.cxx files, is it doing so now for .S??

AthaB{lee49}$ readelf -S setjmp_amd64.S.o | grep .note
  [ 4] .note.GNU-stack   PROGBITS         0000000000000000  000000d6
AthaB{lee49}$ readelf -S setjmp_amd32.S.o | grep .note
  [ 3] .note.GNU-stack   PROGBITS         0000000000000000  00000040
AthaB{lee49}$ readelf -S setjmp_arm64.S.o | grep .note
  [ 3] .note.GNU-stack   PROGBITS         0000000000000000  00000040
AthaB{lee49}$ readelf -S setjmp_x64.S.o | grep .note
  [ 3] .note.GNU-stack   PROGBITS         0000000000000000  00000040

@LeeTibbert
Copy link
Contributor Author

LeeTibbert commented May 21, 2024

PR #3925 provided a quick fix to the base issue.

I think we are probably not up to a thousand cuts on delimcc issues but we are working on it. It would
be nice to bring out the "Big Hammer" and make a more fundamental change.

Whilst working on #3925 it dawned on me that we might be able to avoid the core "presenting useless, weird stuff
to the compiler and then having to fix it up later" problem without a major rework to the build logic. That re-write
is coming soon, but perhaps we can kick the stone down the road once again. The positive spin on that is "defer to
another Evolution".

It occurred to me that we could concatenate the four current files into one new file (well, it will take a little bit of
editing) and the have one, guarded, .note.GNU-stack section at the end.

The compiler would see one file. That file would always generate some code (or else hit a #error "Unknown architecture")
and always have the guarded .note-GNU-stack section.

Yes, this is a slight deviation of from the project where this code was taken. I believe that project is venerable and sees
little change. If an architecture needed to be corrected or a new one added, it would be easy to see the section
which in the OneBigFile.S which needed to change.

I have not tried this out, but I am pretty sure it would work. If encouraged, I could first do the changes in a sandbox
and then, if they appear worthy, submit a PR. I could make that a draft PR if people want to iterate on the idea
(or consider it too ugly a hack to accept).

Please advise,


Later: Please see PR #3926

CI was painfully slow, with intermittent build failures, so I had some time to try out the idea of this entry.
Seemed to work.

At the very least, it should speed up the build a bit, by not considering know-to-be
unused stuff. No more "Why is setjmp_amd32.S" being compiled on a 64 bit system?"

There may or may not be a .note.GNU-stack with various compilers, but it is gone now.

@LeeTibbert
Copy link
Contributor Author

I believe the build errors are intermittent and unrelated to the substance of this PR
(I'm wearing that card out and will need to replace it all to soon. Hard shopping!)

I read the logs but am not going to chase those errors unless I have to. I believe
PR #3926 is a much better approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment