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.

Sign up for GitHub

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

Jump to bottom

Enable LLVM toolchain for building LKL targets. #512

Merged
merged 2 commits into from
Jan 27, 2023
Merged

Enable LLVM toolchain for building LKL targets. #512

merged 2 commits into from
Jan 27, 2023

Conversation

rodionov
Copy link

Initial commit enabling clang for LKL to start with fuzzing.

@rodionov rodionov requested review from tavip and thehajime January 24, 2023 20:39
@rodionov rodionov self-assigned this Jan 24, 2023
@rodionov rodionov force-pushed the master branch 5 times, most recently from 72f4205 to cfef154 Compare January 25, 2023 00:32
@tavip
Copy link
Member

tavip commented Jan 25, 2023

@thehajime any idea why OpenVPN fails to install on the windows target?

@thehajime
Copy link
Member

@thehajime any idea why OpenVPN fails to install on the windows target?

Microsoft Windows [Version 10.0.17763.3887]
(c) 2018 Microsoft Corporation. All rights reserved.

cat: 'c:\msilog.txt': No such file or directory

I have zero idea, sorry. my best guess is that the runner image of windows was updated last week and has some different behavior on the msi installation.

actions/runner-images@74541db

you can delete (or comment out) the particular line on cat c:\\msilog.txt from the yaml file.

@rodionov rodionov force-pushed the master branch 3 times, most recently from 86c3c99 to b46b072 Compare January 26, 2023 04:25
@rodionov
Copy link
Author

Just added a CI target for clang-build test.

The checkpatch emits a warning for use of volatile keyword in the definition of unsigned long wrong_size_cmpxchg(volatile void *ptr) in misc.c. Remove the volatile keyword results in the compile time error due to the mismatch of the function prototypes in definition vs declaration. @thehajime @tavip are you OK with this warning or would like to have it ultimately fixed?

@thehajime
Copy link
Member

@thehajime any idea why OpenVPN fails to install on the windows target?

sorry, it looks like the installation of openvpn isn't triggered at all. Even commenting out the line of interest, make test failed on windows due to lack of openvpn installation.

please revert the changes on the comment out (the cat msi.log part); I will investigate it later independent from this PR.

Thanks.

thehajime
thehajime requested changes Jan 27, 2023
View reviewed changes
Copy link
Member

@thehajime thehajime left a comment

Choose a reason for hiding this comment

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

LGTM.

Please revert the change as I commented (#512 (comment)), other than that I'm fine with this patchset.

As for the warnings on volatile keyword, I'm fine with that too.

arch/lkl/kernel/misc.c Show resolved Hide resolved
@thehajime
Copy link
Member

maybe we can close #478 if this is merged.

@rodionov
Enable LLVM toolchain for building LKL targets.
f664d68 
To build LKL targets with clang/ld.lld provide LLVM and CROSS_COMPILE
variables on the make command line in a similar way they used in the
upstream Linux mainline:

// to make LKL targets with the default versions of clang/ld.lld
make -C tools/lkl LLVM=1 CROSS_COMPILE=x86_64-linux-gnu

// to make LKL targets with clang/ld.lld in a specific location
make -C tools/lkl LLVM=/usr/bin/ CROSS_COMPILE=x86_64-linux-gnu

// to make LKL targets with a specific versions of clang/ld.lld, e.g. 13
make -C tools/lkl LLVM=-13 CROSS_COMPILE=x86_64-linux-gnu

Additional documentation on LLVM/CROSS_COMPILE variables is available in
Documentation/kbuild/llvm.rst.

At the moment the LKL makefiles support x86_64-linux-gnu LLVM target
only.

Signed-off-by: Eugene Rodionov <rodionov@google.com>
@rodionov
github CI: add target for clang-built LKL.
9bc2d46 
Add a new target to build LKL tests with LLVM toolchain using clang/lld.

Signed-off-by: Eugene Rodionov <rodionov@google.com>
@rodionov
Copy link
Author

Thank you, @thehajime! I reverted commented out changes in the yaml file.

tavip
tavip approved these changes Jan 27, 2023
View reviewed changes
Copy link
Member

@tavip tavip left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Eugene!

@tavip tavip merged commit b00f0fb into lkl:master Jan 27, 2023
@thehajime thehajime mentioned this pull request Jun 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thehajime thehajime thehajime approved these changes

@tavip tavip tavip approved these changes

Assignees

@rodionov rodionov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rodionov @tavip @thehajime