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

Support for scan-build (clang static analyzer) #39

Open
Harvie opened this issue Dec 4, 2020 · 26 comments
Open

Support for scan-build (clang static analyzer) #39

Harvie opened this issue Dec 4, 2020 · 26 comments

Comments

@Harvie
Copy link

Harvie commented Dec 4, 2020

Hello,
can you please add support for scan-build in the mongoose os build system?

https://clang-analyzer.llvm.org/scan-build.html

It is easy to use. You just add clang package to docker image and use scan-build to invoke whatever build system you are already using. Eg. instead of make you invoke scan-build make. It will wrap GCC invokations to do clang static analysis as well. It offers interresting warnings and insight into possible issues present in the code, which are otherwise not detected by compiler.

Note that while scan-build is part of clang compiler, it will still use gcc compiler to do the actual compiling unless you configure your system to use clang (which should work as drop-in gcc replacement in case you want to try it as well).

Besides of compiling the code in gcc as usual, this will produce additional warnings printed in the console and will also produce directory containing verbose build protocol to be later examined by user. I think this would allow us to write more robust code.

Unfortunately i cannot simply invoke it as scan-build mos build --local, because the wrapper will obviously not affect build process inside docker container. Only way to make this work is adding the support directly to the docker image.

Perhaps this can be implemented as optional mode for mos, eg. activated as: mos build --local --scan-build

@rojer
Copy link
Contributor

rojer commented Dec 5, 2020

i love static analysis tools, the more we can run on the code the better.
i will take a look at this when i have the time, or if you can do it yourself and put up PRs, that would be even better,

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

i will take a look at this when i have the time, or if you can do it yourself and put up PRs, that would be even better

I might, but TBH i don't even know where to start... Where are the sources of docker image?

@rojer
Copy link
Contributor

rojer commented Dec 5, 2020

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

https://github.com/cesanta/mongoose-os/tree/master/tools/docker

But how do i force mos tool on my machine to use some unofficial development version of docker image? isn't it hardcoded to use image from docker hub?

I am not tremendously experienced with docker, but if i remember correctly it might even be possible to add some kind of "overlay image" which would stack the scan-build layer on top of the mongoose image without need to modify it. But modifiing the image directly might be easier...

@rojer
Copy link
Contributor

rojer commented Dec 5, 2020

it's simple, actually: you build local image under a different tag and change deps/modules/mongoose-os/platforms/PLATFORM/sdk.version to use it.

so, since we are talking ubuntu image, you:

  1. modify Dockerfile
  2. make docker-build DOCKER_TAG=custom
  3. edit sdk.version to use docker.io/mgos/ubuntu-build:custom
  4. build your app

then iterate steps 1, 2, 4

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

https://github.com/cesanta/mongoose-os/blob/master/tools/docker/ubuntu/Dockerfile-ubuntu-build

There seems to be clang and even valgrind already included in the image...
However valgrind does dynamic analysis, so correct me if i am wrong, but i think it cannot be used to analyze ESP32 code in docker on PC...

update: ubuntu probably has scan-build in separate clang-tools package, so the clang package is not enough

@rojer
Copy link
Contributor

rojer commented Dec 5, 2020

yeah, dynamic analysis on the target is not possible. the best we can do is analyze app built for ubuntu platform.

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

the best we can do is analyze app built for ubuntu platform.

What is this ubuntu platform for? is it just for testing or for actual devices? Unfortunately i use mos modules which do not have "ubuntu" listed as platform in mos.yml, so it will probably just refuse to build. Also my code often starts allocating memory only in case it gets some data from hardware peripherals which are not present on ubuntu, so i guess this will not be very helpful in my case... Without some peripherals present it will not even boot.

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

Also when i inspect the dockerfile https://github.com/cesanta/mongoose-os/blob/master/tools/docker/ubuntu/Dockerfile-ubuntu-build

It seems to describe what packages are installed in the image, but there is no mention of any buildscripts being invoked... I've looked at mgos dockerhub account and it seems there are lots of images. eg. this:

https://hub.docker.com/layers/mgos/fwbuild-instance/2.18.0/images/sha256-4a49a533f6a256f0296ad7bf5f1e112c3323f141ae824cab0b0121d8ccd81630?context=explore

seems to specifiy ENTRYPOINT ["/usr/local/bin/fwbuild-instance"] which is probably the script i am looking for. However i can't find it anywhere on github. I would like to find where the actual building is done, so i can wrap that command with scan-build.

@rojer
Copy link
Contributor

rojer commented Dec 5, 2020

build process itself is driven by the make file: https://github.com/cesanta/mongoose-os/blob/master/platforms/ubuntu/Makefile.build

mos runs make -f Makefile.build with a bunch of variables.

docker run --name mos_build_2020-12-05T17-20-14-00_5434562690696808693 --rm -i -v ... a lot of volume mappings ... make '-j' '8' '-C' '/app' '-f' '/home/rojer/allterco/shelly-homekit/deps/modules/mongoose-os/platforms/ubuntu/Makefile.build' 'all' ... while bunch of variables

run mos build --verbose and find the line that starts with "Docker arguments:" for full invocation

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

So perhaps it might be enough to replace
docker run ... /bin/bash -c nice make '-j' '8' '-C' '/app' with
docker run ... /bin/bash -c nice scan-build make '-j' '8' '-C' '/app'

that way we would only need to add the clang-tools package and add small change to mos tool

diff --git a/tools/docker/esp32/Dockerfile-esp32-build b/tools/docker/esp32/Dockerfile-esp32-build
index e7cf83a..871aef0 100644
--- a/tools/docker/esp32/Dockerfile-esp32-build
+++ b/tools/docker/esp32/Dockerfile-esp32-build
@@ -6,7 +6,7 @@ RUN DEBIAN_FRONTEND=noninteractive apt-get update && \
       libexpat-dev libncurses5-dev libtool-bin \
       python python-dev python-git python-pyelftools python-serial python-six python-yaml \
       python3 python3-dev python3-git python3-pyelftools python3-serial python3-six python3-yaml \
-      software-properties-common texinfo unzip wget zip && \
+      software-properties-common texinfo unzip wget zip clang-tools && \
     apt-get clean
 
 RUN cd /tmp && \
diff --git a/cli/build_local.go b/cli/build_local.go
index 135bb7a..d4a481e 100644
--- a/cli/build_local.go
+++ b/cli/build_local.go
@@ -442,7 +442,7 @@ func buildLocal2(ctx context.Context, bParams *build.BuildParams, clean bool) (e
                }
 
                dockerRunArgs = append(dockerRunArgs,
-                       "/bin/bash", "-c", "nice make '"+strings.Join(makeArgs, "' '")+"'",
+                       "/bin/bash", "-c", "nice scan-build make '"+strings.Join(makeArgs, "' '")+"'",
                )
 
                if err := runDockerBuild(dockerRunArgs); err != nil {

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

But when i copy all the arguments printed in mos build --verbose and try to run it using docker command, it just prints 0 and exits.

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

Also i can't build "custom" tag, because it looks for the tag named like that in other repositories...

Step 11/22 : RUN git clone --recursive --branch $DOCKER_TAG https://github.com/mongoose-os/esp-idf /opt/Espressif/esp-idf
 ---> Running in 17dbc7e5fa50
Cloning into '/opt/Espressif/esp-idf'...
fatal: Remote branch custom not found in upstream origin
The command '/bin/sh -c git clone --recursive --branch $DOCKER_TAG https://github.com/mongoose-os/esp-idf /opt/Espressif/esp-idf' returned a non-zero code: 128
make: *** [../docker.mk:24: docker-build-esp32-build] Chyba 128

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

this also failed: make docker-build, because there is no latest in other repos (same way as there is no custom tag)
but i was able to build like this: make docker-build DOCKER_TAG=3.3-r6.

and i was able to run scan-build with modified mos tool as described in #39 (comment)

BUT it seems that scan-build does call different GCC than the one needed in esp-idf (at least it does not force to use clang), because it fails with following error:
gcc: error: unrecognized command line option '-mlongcalls'

-mlongcalls is xtensa specific gcc option, so it obviously tries to use system-wide gcc instead of the esp32 gcc.

So we probably need to figure this out...

@rojer
Copy link
Contributor

rojer commented Dec 5, 2020

Also i can't build "custom" tag, because it looks for the tag named like that in other repositories...

forget about ESP32, it won't work. focus on ubuntu platform for using such tools. ubuntu platform is exactly for that - we don't expect you to deploy apps running on it, but the aim is to make it mostly work, to be able to run a good chunk of your logic under ASAN, UBSAN, valgrind and such.

@rojer
Copy link
Contributor

rojer commented Dec 5, 2020

for example, i do most of my logic developement of the shelly-homekit firmware under Ubuntu - the ShellyU target here.

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

forget about ESP32, it won't work

I think this one might actualy be doable as it's static analysis. it just goes through the sources and then calls actual compiler, which might be the gcc from esp32 toolchain.

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

scan-build has these options, perhaps we need to set them properly:
[--use-c++ [=compiler_path]] [--use-cc [=compiler_path]]

maybe:

--use-cc=/opt/Espressif/xtensa-esp32-elf/bin/xtensa-esp32-elf-gcc
--use-c++=/opt/Espressif/xtensa-esp32-elf/bin/xtensa-esp32-elf-g++

update: this seems to work bit better, but still does not build... i am not sure if this affects the linker as well...

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

I guess i am giving up for now :-(

for example, i do most of my logic developement of the shelly-homekit firmware under Ubuntu

But when i do mos build --local --platform=ubuntu the build fails basicaly on any module which contains build_vars

@rojer
Copy link
Contributor

rojer commented Dec 5, 2020

if it's a pre-compilation step anyway, all you need to do is just amke your project build for --platform=ubuntu, not necessarily run.
this can be usually be achieved by removing some platform restrictions in manifests and adding a bunch of stubs.

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

I guess the ubuntu build might be the way as long as i am able to build whole codebase.

Also i recently started running out of disk space and i can't fit both docker images to my laptop :-D These are relatively big, so i would preffer to only have one...

@rojer
Copy link
Contributor

rojer commented Dec 5, 2020

oh man :) get yourself a nice, big SSD :)

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

oh man :) get yourself a nice, big SSD :)

haha. i already have SSD, but i am considering laptop upgrade in next year, so i don't see point in buying new SATA drive, when my next laptop is unlikely to use SATA...

@Harvie
Copy link
Author

Harvie commented Dec 5, 2020

Anyway... i figured out another two options for analyzing directly during xtensa build...

1.) Get scan-build support directly by esp-idf: espressif/esp-idf#6213
2.) Upgrade to gcc 10 which has recently integrated static analyzer as wel (but the -fanalzyer feature set is currently basic and perhaps gcc10 does not even have support for esp-idf or xtensa): espressif/esp-idf#6214

https://developers.redhat.com/blog/2020/03/26/static-analysis-in-gcc-10/
https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html

@Harvie
Copy link
Author

Harvie commented Jun 11, 2022

ESP-IDF just added support for GCC 11: espressif/esp-idf#6214
Which contains static analyzer as well. However it is considered to be complementary to do stuff that other analyzers cannot do. So it still makes sense to use both GCC11 -fanalyzer and clang-scan.

@Harvie
Copy link
Author

Harvie commented Jan 9, 2023

Also ESP-IDF just added option to do statical analysis using clang: espressif/esp-idf#6213

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

2 participants