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

feat: Added cheribsd preset #358

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

feat: Added cheribsd preset #358

wants to merge 20 commits into from

Conversation

mereacre
Copy link
Contributor

@mereacre mereacre commented Dec 6, 2022

Adds a new cheribsd preset with a different set of C flags specific for Cheri architecture in purecap mode.

@mereacre mereacre requested a review from aloisklink December 6, 2022 14:38
@codecov
Copy link

codecov bot commented Dec 6, 2022

Codecov Report

Merging #358 (de84d93) into main (4d5daaa) will increase coverage by 0.19%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##             main     #358      +/-   ##
==========================================
+ Coverage   51.79%   51.99%   +0.19%     
==========================================
  Files         139      138       -1     
  Lines       19315    19239      -76     
==========================================
- Hits        10005    10004       -1     
+ Misses       9310     9235      -75     
Impacted Files Coverage Δ
src/runctl.c 53.49% <33.33%> (-0.44%) ⬇️
tests/test_edgesec.c 94.53% <0.00%> (-0.79%) ⬇️
src/sqlhook.c

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

This PR is referencing a CMakeModules/CMakeToolchains/cheri.cmake file, but it's not in this PR.

CMakePresets.json Outdated Show resolved Hide resolved
set(CMAKE_C_COMPILER clang)
set(CMAKE_CXX_COMPILER clang++)

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -target aarch64-unknown-freebsd -march=morello+c64 -mabi=purecap -Xclang -morello-vararg=new")
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

I'll trust you on that the toolchain file works properly :)

I might need to wait until Friday until I get to my home computer to test this.

@mereacre mereacre requested a review from aloisklink December 6, 2022 18:09
conftest.c Outdated
Comment on lines 1 to 16
/* confdefs.h */
#define PACKAGE_NAME "util-linux"
#define PACKAGE_TARNAME "util-linux"
#define PACKAGE_VERSION "UNKNOWN"
#define PACKAGE_STRING "util-linux UNKNOWN"
#define PACKAGE_BUGREPORT "[email protected]"
#define PACKAGE_URL "http://www.kernel.org/pub/linux/utils/util-linux/"
/* end confdefs.h. */

int
main (void)
{

;
return 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this isn't used, so we can delete it.

Copy link
Contributor

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

Looks good! Congratulations on getting it working.

@aloisklink
Copy link
Contributor

aloisklink commented Dec 9, 2022

Hmmmm, a bit weird. It looks like the Debian build is failing to the following error:

[ 23%] Linking C executable test_wrap_log_error
cd /build/edgesec-0.1.0-alpha.0/obj-x86_64-linux-gnu/tests/utils && /usr/bin/cmake -E cmake_link_script CMakeFiles/test_wrap_log_error.dir/link.txt --verbose=1
/usr/bin/cc -g -O2 -ffile-prefix-map=/build/edgesec-0.1.0-alpha.0=. -flto=auto -ffat-lto-objects -flto=auto -ffat-lto-objects -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -Wunused-variable -Wall -Wextra -O3 -DNDEBUG -Wl,-Bsymbolic-functions -flto=auto -ffat-lto-objects -flto=auto -Wl,-z,relro -Wl,--wrap=log_levels CMakeFiles/test_wrap_log_error.dir/test_wrap_log_error.c.o CMakeFiles/wrap_log_error.dir/wrap_log_error.c.o -o test_wrap_log_error  ../../src/utils/liblog.a /usr/lib/x86_64-linux-gnu/libcmocka.so ../../src/utils/liblog.a /usr/lib/x86_64-linux-gnu/libcmocka.so 
/usr/bin/ld: /tmp/ccM8V57b.ltrans0.ltrans.o: in function `__wrap_log_levels':
./obj-x86_64-linux-gnu/tests/utils/./tests/utils/wrap_log_error.c:27: undefined reference to `log_levels'

I'm not 100% sure why, since I'm pretty sure the only thing you're changing is this line

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wunused-variable -Wall -Wextra")

Edit: I've made a new branch called build/fix-CMAKE_C_FLAGS (commit 4b663a0) that only changes the CMAKE_C_FLAGS! Hopefully that should fix this issue (at least, once I get it working). Edit2: PR here #364

@mereacre mereacre added this to the CheriBSD Support milestone Dec 12, 2022
@aloisklink
Copy link
Contributor

By the way, @mereacre, this is the test errors that we're getting on the freebsd preset:

me@me:~/edgesec$ ctest --preset freebsd --output-on-failure --exclude-regex "$exclude_regex"
...
 2/40 Test  #2: test_runctl ......................***Failed    0.04 sec
[==========] tests: Running 2 test(s).
[ RUN      ] test_init_context
2022-12-13 13:50:16.697  �[36mDEBUG�[0m�[0m �[90mrunctl.c:257:�[0m Getting system commands paths
2022-12-13 13:50:16.697  �[36mDEBUG�[0m�[0m �[90mrunctl.c:321:�[0m Opening the macconn db=:memory:
2022-12-13 13:50:16.718  �[36mDEBUG�[0m�[0m �[90mrunctl.c:328:�[0m Creating subnet to interface mapper...
2022-12-13 13:50:16.718  �[36mDEBUG�[0m�[0m �[90mrunctl.c:339:�[0m Creating VLAN ID to interface mapper...
[       OK ] test_init_context
[ RUN      ] test_run_engine
2022-12-13 13:50:16.718  �[36mDEBUG�[0m�[0m �[90mrunctl.c:257:�[0m Getting system commands paths
2022-12-13 13:50:16.718  �[36mDEBUG�[0m�[0m �[90mrunctl.c:321:�[0m Opening the macconn db=:memory:
2022-12-13 13:50:16.719  �[36mDEBUG�[0m�[0m �[90mrunctl.c:328:�[0m Creating subnet to interface mapper...
2022-12-13 13:50:16.719  �[36mDEBUG�[0m�[0m �[90mrunctl.c:339:�[0m Creating VLAN ID to interface mapper...
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:430:�[0m AP name: 
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:431:�[0m AP interface: 
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:454:�[0m Setting the ip forward os system flag...
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:461:�[0m Adding default mac mappers...
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:468:�[0m Creating subnet interfaces...
2022-12-13 13:50:16.719  �[36mDEBUG�[0m�[0m �[90miface.c:284:�[0m Commiting interface changes
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:479:�[0m Creating supervisor on  with port 0
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:488:�[0m Looking for VLAN capable wifi interface...
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:520:�[0m Creating the radius server on port 0 with client ip 
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:530:�[0m Running the dhcp service...
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:541:�[0m Running the mdns forwarder service thread...
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:551:�[0m ++++++++++++++++++
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:552:�[0m Running event loop
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:553:�[0m ++++++++++++++++++
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:556:�[0m Exit event loop
2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90meloop.c:655:�[0m eloop_get_sock_table: eloop data ptr was NULL
[  ERROR   ] --- Test failed with exception: Bus error(10)
[  FAILED  ] test_run_engine
[==========] tests: 2 test(s) run.
[  PASSED  ] 1 test(s).
[  FAILED  ] tests: 1 test(s), listed below:
[  FAILED  ] test_run_engine

 1 FAILED TEST(S)

I'm guessing it is to do with how you changed ip to ifconfig in https://github.com/nqminds/edgesec/blob/de84d93d38bfd843770ac36e131090d1c1845222/src/runctl.c, but I don't know enough about 🚌 to know why there is a 🚌 error.

Maybe we have an uninitialized pointer somewhere, or there are alignment issues?

@mereacre
Copy link
Contributor Author

mereacre commented Feb 3, 2023

 Bus Error (also known as SIGBUS and is usually signal 10) occur when a process is trying to access memory that the CPU cannot physically address...

I believe that after

2022-12-13 13:50:16.719  �[32mINFO �[0m�[0m �[90mrunctl.c:556:�[0m Exit event loop

runctl is trying to free up memory and accessed something that was already freed and therefore it gives the Bus(10) error. We need to replicate this error on other presets.

@aloisklink
Copy link
Contributor

runctl is trying to free up memory and accessed something that was already freed and therefore it gives the Bus(10) error. We need to replicate this error on other presets.

Hmmm, I think we don't actually ever test the "USE_GENERIC_IP_SERVICE": true syntax except for the freebsd preset.

The recap preset does use USE_GENERIC_IP_SERVICE:

"name": "recap",
"inherits": "default",
"displayName": "recap tool",
"description": "recap tool",
"cacheVariables": {
"BUILD_MNL_LIB": false,
"BUILD_NETLINK_LIB": false,
"USE_NETLINK_SERVICE": false,
"USE_UCI_SERVICE": false,
"USE_GENERIC_IP_SERVICE": true,

However, there is a recap build preset, but no recap test preset.

@mereacre
Copy link
Contributor Author

mereacre commented Feb 3, 2023

The USE_GENERIC_IP_SERVICE uses the ip command (by executing the process) instead of libnl. So, that shouldn't work on freebsd because it uses ifconfig.

@aloisklink
Copy link
Contributor

aloisklink commented Feb 3, 2023

The tests were passing previously, though. Maybe edgesec had better error handling for when ip wasn't found, but doesn't know what to do with ifconfig?

My gut feeling is to undo the src/runctl.c changes so that we can merge this PR, and maybe make them in a new PR called "Use ifconfig for ip service", but it's up to you.

Edit: I usually use https://github.com/CTSRD-CHERI/cheribuild to do cross-compiling, so I'm probably not going to use this cheribsd preset.

@mereacre
Copy link
Contributor Author

mereacre commented Feb 3, 2023

Which changes do you want to undo?

The USE_GENERIC_IP_SERVICE was added a year ago as I remember.

@aloisklink
Copy link
Contributor

Which changes do you want to undo?

These changes:

https://github.com/nqminds/edgesec/pull/358/files#diff-2952f7788619c2fe81d6536e6b254d97a9e46b87e2f7d54a4b7e15e5624aad6eR245-R270

Essentially, we should keep src/runctl.c how it is on the main branch, and not change it in this PR. We can then update src/runctl.c to work with ifconfig in a separate PR.

Copy link
Contributor Author

@mereacre mereacre left a comment

Choose a reason for hiding this comment

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

Ok

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