On 27 Nov 2023, at 19:18, Ilya Maximets wrote:
> On 11/27/23 18:36, Eelco Chaudron wrote:
>> This patch detects new static analyze issues, and report them.
>> It does this by reporting on the delta for this branch, compared
>> to the previous branch.
>>
>> For example the error might look like this:
>>
>> error level: +0 -0 no changes
>> warning level: +2 +0
>> New issue "deadcode.DeadStores Value stored to 'remote' is never read"
>> (1 occurrence)
>> file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:86
>> New issue "unix.Malloc Potential leak of memory pointed to by 'remote'"
>> (1 occurrence)
>> file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:95
>> note level: +0 -0 no changes
>> all levels: +2 +0
>>
>> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
>> ---
>>
>> changes in v2:
>> - When it's a new branch, it compares it to the HEAD of the default branch.
>>
>> .ci/linux-build.sh | 29 ++++++++++
>> .github/workflows/build-and-test.yml | 96
>> ++++++++++++++++++++++++++++++++++
>> 2 files changed, 125 insertions(+)
>>
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index aa2ecc505..fedf1398a 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -49,6 +49,30 @@ function build_ovs()
>> make -j4
>> }
>>
>> +function clang_analyze()
>> +{
>> + [ -d "./base-clang-analyzer-results" ] && cache_build=false \
>> + || cache_build=true
>> + if [ "$cache_build" = true ]; then
>> + # If this is a cache build, proceed to the base branch's directory.
>> + cd base_ovs_main
>> + fi;
>> +
>> + configure_ovs $OPTS
>> + make clean
>> + scan-build -o ./clang-analyzer-results -sarif --use-cc=clang make -j4
>> +
>> + if [ "$cache_build" = true ]; then
>> + # Move results, so it will be picked up by the cache.
>> + mv ./clang-analyzer-results ../base-clang-analyzer-results
>> + cd ..
>> + else
>> + # Only do the compare on the none cache builds.
>> + sarif --check note diff ./base-clang-analyzer-results \
>> + ./clang-analyzer-results
>> + fi;
>> +}
>> +
>> if [ "$DEB_PACKAGE" ]; then
>> ./boot.sh && ./configure --with-dpdk=$DPDK && make debian
>> mk-build-deps --install --root-cmd sudo --remove debian/control
>> @@ -116,6 +140,11 @@ fi
>>
>> OPTS="${EXTRA_OPTS} ${OPTS} $*"
>>
>> +if [ "$CLANG_ANALYZE" ]; then
>> + clang_analyze
>> + exit 0
>> +fi
>> +
>> if [ "$TESTSUITE" = 'test' ]; then
>> # 'distcheck' will reconfigure with required options.
>> # Now we only need to prepare the Makefile without sparse-wrapped CC.
>> diff --git a/.github/workflows/build-and-test.yml
>> b/.github/workflows/build-and-test.yml
>> index 09654205e..d15105e7d 100644
>> --- a/.github/workflows/build-and-test.yml
>> +++ b/.github/workflows/build-and-test.yml
>> @@ -223,6 +223,102 @@ jobs:
>> name: logs-linux-${{ join(matrix.*, '-') }}
>> path: logs.tgz
>>
>> + build-clang-analyze:
>> + needs: build-dpdk
>> + env:
>> + dependencies: |
>> + automake bc clang-tools libbpf-dev libnuma-dev libpcap-dev \
>> + libunbound-dev libunwind-dev libssl-dev libtool llvm-dev \
>> + python3-unbound
>> + CC: clang
>> + DPDK: dpdk
>> + CLANG_ANALYZE: true
>> + name: clang-analyze
>> + runs-on: ubuntu-22.04
>> + timeout-minutes: 30
>> +
>> + steps:
>> + - name: checkout
>> + uses: actions/checkout@v3
>> +
>> + - name: get base branch sha
>> + id: base_branch
>> + run: |
>> + if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
>> + echo "sha=$BASE_SHA" >> $GITHUB_OUTPUT
>> + else
>> + if [ "$EVENT_BEFORE" = "0000000000000000000000000000000000000000"
>> ]; then
>> + echo "sha=$DEFAULT_BRANCH" >> $GITHUB_OUTPUT
>
> How this is going ot work on patches for older branches?
Good question, it’s not :( Took a little bit to figure out how to do this, as
we have no reference branch. The solution I came up with was to figure out all
parent branches, and use the most recent main/master or branch-x.x one. So it
looks like this:
if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
echo "sha=$BASE_SHA" >> $GITHUB_OUTPUT
else
if [ "$EVENT_BEFORE" = "0000000000000000000000000000000000000000" ];
then
set sha=$(git log --simplify-by-decoration --decorate=full \
--pretty=format:'%d' | \
grep -oP 'refs/remotes/origin/\K[^, )]+' | \
grep -m1 -E '^master$|^main$|^branch-[0-9]+\.[0-9]+$')
[ -z "$sha" ] && echo "sha=$DEFAULT_BRANCH" >> $GITHUB_OUTPUT \
|| echo "sha=$sha" >> $GITHUB_OUTPUT
else
echo "sha=$EVENT_BEFORE" >> $GITHUB_OUTPUT
fi
fi
>> + else
>> + echo "sha=$EVENT_BEFORE" >> $GITHUB_OUTPUT
>> + fi
>> + fi
>> + env:
>> + BASE_SHA: ${{ github.event.pull_request.base.sha }}
>> + EVENT_BEFORE: ${{ github.event.before }}
>> + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
>
> Can we swap env and run sections, so everything that is used
> is already defined?
Actually, the order does not matter as the env is set before run in the same
step, but I will swap as it’s more easy to understand/read what is going on.
I’ll try to send out a v2 later this week.
>> +
>> + - name: checkout base branch
>> + uses: actions/checkout@v3
>> + with:
>> + ref: ${{ steps.base_branch.outputs.sha }}
>> + path: base_ovs_main
>> +
>> + - name: update PATH
>> + run: |
>> + echo "$HOME/bin" >> $GITHUB_PATH
>> + echo "$HOME/.local/bin" >> $GITHUB_PATH
>> +
>> + - name: generate cache key
>> + id: cache_key
>> + run: |
>> + echo "key=clang-analyze-$(git -C base_ovs_main rev-parse HEAD)" \
>> + >> $GITHUB_OUTPUT
>> +
>> + - name: check for analyzer result cache
>> + id: clang_cache
>> + uses: actions/cache@v3
>> + with:
>> + path: base-clang-analyzer-results
>> + key: ${{ steps.cache_key.outputs.key }}
>> +
>> + - name: set up python
>> + uses: actions/setup-python@v4
>> + with:
>> + python-version: '3.9'
>> +
>> + - name: get cached dpdk-dir
>> + uses: actions/cache/restore@v3
>> + with:
>> + path: dpdk-dir
>> + key: ${{ needs.build-dpdk.outputs.dpdk_key }}
>> +
>> + - name: update APT cache
>> + run: sudo apt update || true
>> +
>> + - name: install common dependencies
>> + run: sudo apt install -y ${{ env.dependencies }}
>> +
>> + - name: install sarif tools
>> + run: sudo pip3 install --disable-pip-version-check sarif-tools
>> +
>> + - name: prepare
>> + run: ./.ci/linux-prepare.sh
>> +
>> + - name: build base reference
>> + if: steps.clang_cache.outputs.cache-hit != 'true'
>> + run: ./.ci/linux-build.sh
>> +
>> + - name: build
>> + run: ./.ci/linux-build.sh
>> +
>> + - name: save cache
>> + uses: actions/cache/save@v3
>> + if: always() && steps.clang_cache.outputs.cache-hit != 'true'
>> + with:
>> + path: base-clang-analyzer-results
>> + key: ${{ steps.cache_key.outputs.key }}
>> +
>> build-osx:
>> env:
>> CC: clang
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
