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

devel/llvm-base: switch to init-all=zero #153

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

brooksdavis
Copy link
Member

Zero bits of the stack the compiler can't prove are initialized before use or escape by default.

Issue: CTSRD-CHERI/cheribsd#2045
See also: CTSRD-CHERI/cheribsd#2046

@brooksdavis
Copy link
Member Author

brooksdavis commented Mar 14, 2024

This is completely untested since I did it on a non-morello system, but I think it's roughly correct.

Copy link
Member

@kwitaszczyk kwitaszczyk left a comment

Choose a reason for hiding this comment

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

With llvm-morello-13.0.d20230914 (the current llvm-morello version in packages for 23.11) and the suggested fix in wrapper.sh.in, I get:

$ sh -x /usr/bin/cc -o test test.c
(...)
+ /usr/local64/bin/clang '-ftrivial-auto-var-init=zero' '-march=morello' '-mabi=purecap' -Xclang '-morello-vararg=new' -Xclang '-morello-bounded-memargs=caller-only' -o test test.c
clang-13: error: -ftrivial-auto-var-init=zero hasn't been enabled. Enable it at your own peril for benchmarking purpose only with -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

Note that my llvm-morello version is outdated as we already have a newer version in ports.

@brooksdavis
Copy link
Member Author

With llvm-morello-13.0.d20230914 (the current llvm-morello version in packages for 23.11) and the suggested fix in wrapper.sh.in, I get:

$ sh -x /usr/bin/cc -o test test.c
(...)
+ /usr/local64/bin/clang '-ftrivial-auto-var-init=zero' '-march=morello' '-mabi=purecap' -Xclang '-morello-vararg=new' -Xclang '-morello-bounded-memargs=caller-only' -o test test.c
clang-13: error: -ftrivial-auto-var-init=zero hasn't been enabled. Enable it at your own peril for benchmarking purpose only with -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

Note that my llvm-morello version is outdated as we already have a newer version in ports.

I wonder if it makes sense to patch out -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang. I can add the flag in the script, but in the future we'll run into

clang: warning: the flag '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang' has been deprecated and will be ignored [-Wunused-command-line-argument]

@kwitaszczyk
Copy link
Member

I wonder if it makes sense to patch out -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang. I can add the flag in the script, but in the future we'll run into

clang: warning: the flag '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang' has been deprecated and will be ignored [-Wunused-command-line-argument]

If I understand correctly, Clang 16 and newer don't require -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang to enable -ftrivial-auto-var-init=zero. I wonder if LLVM for Morello could cherry-pick that change. Otherwise, we'd have to patch it in CheriBSD ports and cheribuild, which would be ideal to avoid.

With -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang required, it'd be annoying to our users to always include this flag when executing a compiler from LOCALBASE64 instead of cc. The list of required compiler flags is already quite long.

Zero bits of the stack the compiler can't prove are initialized before
use or escape by default.

Issue:		CTSRD-CHERI/cheribsd#2045
@kwitaszczyk
Copy link
Member

I've discussed with @rwatson that we should consider this for the next release but not for the CPM meeting.

Once CTSRD-CHERI/cheribsd#2046 is merged into dev, we can merge this PR, bump the CheriBSD ABI version and rebuild packages again. Note that #141 updated llvm-base, including its versioning format as we discussed there.

@kwitaszczyk kwitaszczyk marked this pull request as draft July 1, 2024 13:44
@kwitaszczyk
Copy link
Member

I've converted this to a draft as we're not planning to use it now.

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.

2 participants