Skip to content

recompile ./xip_ram_perms/xip_ram_perms.elf requires PICOTOOL_NO_LIBUSB #139

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

Closed
josch opened this issue Sep 9, 2024 · 12 comments
Closed

Comments

@josch
Copy link
Contributor

josch commented Sep 9, 2024

Hi,

for the Debian packaging of picotool 2.0, I'd like to rebuild the file ./xip_ram_perms/xip_ram_perms.elf but when reading CMakeLists.txt it seems that doing so is only possible if one also sets PICOTOOL_NO_LIBUSB=1. But I'd like to build picotool with libusb support. Am I misreading something? Can I somehow build picotool with libusb and rebuild ./xip_ram_perms/xip_ram_perms.elf?

Also, why is ./xip_ram_perms/xip_ram_perms.elf tracked in git?

@lurch
Copy link
Contributor

lurch commented Sep 9, 2024

Already fixed by c4b8630 which is part of #131

EDIT: Oh sorry, I misread PICOTOOL_NO_LIBUSB as PICO_NO_PICOTOOL 🤦‍♂️

@will-v-pi
Copy link
Contributor

I think you're misreading the code - it's the other way round, so it only compiles xip_ram_perms.elf if PICOTOOL_NO_LIBUSB=0 (or isn't defined), and USE_PRECOMPILED=false must also be set. I've merged #131 now, which fixes it downloading and building picotool inside the xip_ram_perms build

The reason for including it in the git repository is that users who don't wish to compile from source (for example if they don't have a working pico-sdk build) should still be able to build picotool. This same reasoning applies to the picoboot_flash_id/flash_id.bin build

@will-v-pi
Copy link
Contributor

You can see this job for an example compilation with libusb and compiling these binaries from source

@josch
Copy link
Contributor Author

josch commented Sep 9, 2024

Thank you, it seems that I followed a red-herring and mis-read the if-condition in CMakeLists.txt. I'm currently configuring picotool with -DPICO_SDK_PATH=/usr/src/pico-sdk -DUSE_PRECOMPILED=0 and it seems to work because it wants to build pico-sdk now... which fails becaus:

CMake Warning at /usr/src/pico-sdk/tools/Findpicotool.cmake:28 (message):
  No installed picotool with version 2.0.0 found - building from source

  It is recommended to build and install picotool separately, or to set
  PICOTOOL_FETCH_FROM_GIT_PATH to a common directory for all your SDK
  projects

I'm building picotool 2.0, so obviously it is not available yet. In the pico-sdk 2.0 packaging I solve this by passing -DPICO_NO_PICOTOOL=ON. What is the canonical way to do that with pico-sdk as built by picotool? How do I pass extra cmake flags to pico_sdk_init or whatever the correct place for that is? Thank you!

@will-v-pi
Copy link
Contributor

will-v-pi commented Sep 9, 2024

That should have been fixed in #131 (as I noticed it while writing the tests) which I've just merged into develop, which adds set(PICO_NO_PICOTOOL 1) into the CMakeLists.txt files for xip_ram_perms and picoboot_flash_id - specifically this commit

@josch
Copy link
Contributor Author

josch commented Sep 9, 2024

Thank you, I patched xip_ram_perms/CMakeLists.txt according to that patch. It helps to be able to link this commit in your git repo to allow tracking this change.

Another problem I'm running into is CFLAGS. In Debian, we apply numerous hardening features which of course we'd like to apply to the picotool binary. But those CFLAGS are also propagated to the pico-sdk build and make that build fail because these flags are not supported by the arm-none-eabi compiler and leads to errors like this:

/usr/bin/arm-none-eabi-gcc   -g -O2 -Werror=implicit-function-declaration -ffile-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -fstack-clash-protection -Wformat -Werror=format-security -mbranch-protection=standard  -mcpu=cortex-m33 -mthumb -march=armv8-m.main+fp+dsp -mfloat-abi=softfp -mcmse  -o CMakeFiles/cmTC_ba6a8.dir/testCCompiler.c.obj -c /<<PKGBUILDDIR>>/obj-aarch64-linux-gnu/xip_ram_perms/CMakeFiles/CMakeScratch/TryCompile-qUQY2W/testCCompiler.c
    /tmp/ccN4D761.s: Assembler messages:
    /tmp/ccN4D761.s:42: Error: selected processor does not support `bti' in Thumb mode

Do you know of a good way to pass a different set of compiler flags to the pico-sdk build than to the build of picotool itself? Thanks!

@will-v-pi
Copy link
Contributor

If you pass them to the picotool cmake configure step as -D CMAKE_<LANG>_FLAGS rather than setting the CFLAGS environment variable, then those flags will only be set for the picotool build and not the external projects inside (eg xip_ram_perms, otp_header_parser).

Alternatively you could patch the xip_ram_perms CMakeLists.txt to unset CFLAGS within the file - this will probably need to be done before the include(pico_sdk_import.cmake) line. I think we'd be happy to merge a PR with that patch, as it makes sense to unset any CFLAGS/CXXFLAGS environment variables before compiling xip_ram_perms as they are unlikely to apply to that compilation

@josch
Copy link
Contributor Author

josch commented Sep 9, 2024

Thank you for all your input. I filed #140 to continue discussion about this topic there.

My next issue are embedded build paths in /usr/bin/picotool. Embedded build paths hamper reproducibility (see https://reproducible-builds.org/) and are a privacy issue if the user who built them publishes them without knowing that the binary includes more information from their system than what is contained in the sources itself. For example, the current binary in git contains multiple references to paths under /home/willvpi/amy-stuff/.

Specifically, the embedded paths seem to be of the nature of:

  • ${CMAKE_CURRENT_SOURCE_DIR}/xip_ram_perms
  • ${CMAKE_CURRENT_BINARY_DIR}/xip_ram_perms
  • ${CMAKE_CURRENT_SOURCE_DIR}/xip_ram_perms/set_perms.c

Maybe it could be made possible to either disable the inclusion of local paths with a flag or to replace them with relative paths instead of absolute paths?

@lurch
Copy link
Contributor

lurch commented Sep 9, 2024

Maybe it could be made possible to either disable the inclusion of local paths with a flag or to replace them with relative paths instead of absolute paths?

It's probably worth creating a separate issue for discussing that...

EDIT: Ooops, ignore me! 🤦‍♂️ See William's comment below.

@will-v-pi
Copy link
Contributor

No, it's fully related to this - the paths all come from the xip_ram_perms debug info

will-v-pi added a commit that referenced this issue Sep 9, 2024
This embedded local paths of the build machine into the elf file, which was undesirable behaviour
@will-v-pi
Copy link
Contributor

Should be fixed with that referenced commit - the pico-sdk build adds debug info (-g) to all builds by default, so the PICO_DEBUG_INFO_IN_RELEASE option needs to be turned off to disable that

@josch
Copy link
Contributor Author

josch commented Sep 9, 2024

Perfect, that fixed it! Thanks a bunch!

I think my package is ready now and I have no further questions. I'm thus closing this issue.

Thank you once again!

@josch josch closed this as completed Sep 9, 2024
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

No branches or pull requests

3 participants