Skip to content

Don't generate #! headers over 127 characters#8622

Merged
xavierleroy merged 1 commit intoocaml:trunkfrom
dra27:shebang
May 2, 2019
Merged

Don't generate #! headers over 127 characters#8622
xavierleroy merged 1 commit intoocaml:trunkfrom
dra27:shebang

Conversation

@dra27
Copy link
Copy Markdown
Member

@dra27 dra27 commented Apr 17, 2019

#! lines should not exceed 128 characters, including the null terminator. On Linux, breaking this has confusing consequences:

dra@cosmic:~/ocaml$ ./hello
bash: ./hello: /home/dra/local/aaaaaaaaaa/bbbbbbbbbb/cccccccccc/dddddddddd/eeeeeeeeee/ffffffffff/gggggggggg/hhhhhhhhhh/iiiiiiiiii/jjjjjjjjjj: bad interpreter: Permission denied

This PR detects the worst case in configure and reverts to this pattern:

#!/bin/sh
exec "/full/and/very/long/absolute/path/to/ocamlrun" "$0" "$@"

The same logic is applied by ocamlc for the -use-runtime flag. I fear for @gasche's sanity if we attempt to embed the strlen function as a GNU make macro, so I've chosen to determine statically in configure whether the path to ocamlrun/ocamlrund/ocamlruni might be too long, rather than dynamically determine it in Makefile.

This issue, coupled with a related problem in opam, has caused considerable pain for @ctk21 and our searches showed that this may have hit other packagers of OCaml in the past. It's comparatively easy to hit this scenario when building opam switches with lots of +variant additions or fakeroot package builds, etc.

I don't propose this for 4.08, although I think it might be worth back-porting to opam-repository once the fix is accepted here.

A #! line should not exceed 128 characters (including the \0
terminator). This adds a test - both to the generation of the camlheader
files and also to the -use-runtime flag which falls back to #!/bin/sh
and uses exec to invoke the the interpreter.
@xavierleroy
Copy link
Copy Markdown
Contributor

I learned another silly limitation today :-) For the record, some documentation about #!: https://www.in-ulm.de/~mascheck/various/shebang/#length

@xavierleroy
Copy link
Copy Markdown
Contributor

Thank you @dra27 for the patch and @stedolan for the review. Let's merge!

@xavierleroy xavierleroy merged commit 8838dc7 into ocaml:trunk May 2, 2019
@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Jun 19, 2019

Just following up #2309 (comment) here. There is a problem with this patch, in that the "TODO" comment is more urgent than I realised. So the problem is that this fix does not really work - it needs a little work which means it will be better left out of 4.09 and hopefully be fixed up completely for 4.10

(* shebang mustn't exceed 128 including the #! and \0 *)
if String.length runtime > 125 then
"/bin/sh\n\
exec \"" ^ runtime ^ "\" \"$0\" \"$@\""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use Filename.quote here to guard against paths with " in them.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better late than never, but this will be addressed in the set of PRs for allowing the compiler to be relocated.

As it happens, it's not a bug at the moment inasmuch as you can't get here because Symtable.init doesn't quote the runtime either. Fix will be ready before 4.13.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants