Fix ASAN build clobbering packages after build on the same system#13
Fix ASAN build clobbering packages after build on the same system#13
Conversation
Signed-off-by: Connor Roos <croos@nvidia.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the SONiC build system to better support AddressSanitizer (ASAN) builds alongside regular builds by introducing ASAN-suffixed artifact naming and adjusting how debs/docker images are produced and cached.
Changes:
- Rename key deb/docker outputs for ASAN builds (e.g.,
swss-asan,syncd-asan,docker-orchagent-asan,docker-syncd-*-asan) to avoid collisions with regular build artifacts. - Teach the dpkg deb move step to support renaming via
*_DPKG_DEB_NAME, enabling “target name != dpkg output filename”. - Update several
*_DEP_FLAGSdefinitions to drop$(ENABLE_ASAN)so dependency/cache keys align with the new artifact naming approach.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
slave.mk |
Adds deb renaming support via *_DPKG_DEB_NAME during artifact move; exports ENABLE_ASAN for docker Jinja rendering. |
rules/sysmgr.dep |
Removes $(ENABLE_ASAN) from sysmgr dep flags. |
rules/syncd.mk |
Introduces ASAN-suffixed syncd deb names and dpkg output name mapping. |
rules/syncd.dep |
Removes $(ENABLE_ASAN) from syncd dep flags. |
rules/swss.mk |
Introduces ASAN-suffixed swss deb names and dpkg output name mapping. |
rules/swss.dep |
Removes $(ENABLE_ASAN) from swss dep flags. |
rules/docker-orchagent.mk |
Adds ASAN-suffixed orchagent docker image name and extra deps when ASAN enabled. |
rules/docker-orchagent.dep |
Removes $(ENABLE_ASAN) from orchagent docker dep flags. |
platform/template/docker-syncd-bookworm.mk |
Adds ASAN-suffixed syncd docker base image naming. |
platform/mellanox/docker-syncd-mlnx.mk |
Adds extra dbg deps for ASAN syncd docker image. |
platform/mellanox/docker-syncd-mlnx.dep |
Removes $(ENABLE_ASAN) from Mellanox syncd docker dep flags. |
|
I suppose this also must be changed, we need to add extra package here swss-asan, syncd-asan target https://github.com/sonic-net/sonic-swss/blob/master/debian/control https://github.com/sonic-net/sonic-sairedis/blob/master/debian/control |
|
@copilot make the change vivek asked for |
Signed-off-by: Connor Roos <croos@nvidia.com>
Signed-off-by: Connor Roos <croos@nvidia.com>
Signed-off-by: Connor Roos <croos@nvidia.com>
Signed-off-by: Connor Roos <croos@nvidia.com>
Signed-off-by: Connor Roos <croos@nvidia.com>
Signed-off-by: Connor Roos <croos@nvidia.com>
|
LGTM, do you have sonic-swss and sonic-sairedis PR's? |
Added them to the description |
…net#25643) * [build] Add build timing report and dependency analysis tools Add three scripts for build performance instrumentation: - scripts/build-timing-report.sh: Parse per-package timing from build logs (HEADER/FOOTER timestamps), generate sorted duration table, phase breakdown, parallelism timeline, and CSV export. - scripts/build-dep-graph.py: Parse rules/*.mk dependency graph, compute critical path, fan-out/fan-in bottleneck analysis, and generate DOT/JSON output for visualization. - scripts/build-resource-monitor.sh: Sample CPU, memory, disk I/O, and Docker container count during builds for resource utilization analysis. Add "make build-report" target to slave.mk that runs the timing report and dependency analysis after a build completes. Example output from a VS build on 24-core/30GB machine: - 210 packages built in 53m wall time (173m CPU) - Max concurrency: 5 (with SONIC_CONFIG_BUILD_JOBS=4) - Critical path: 14 packages deep (libnl -> libswsscommon -> utilities) - Top bottleneck: LIBSWSSCOMMON with 48 downstream dependents Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com> * Address Copilot review: fix 17 bugs in build analysis scripts - Use free -m with division instead of free -g to avoid rounding (#1) - Add = and ?= to Makefile dependency regex patterns (#2, #7) - CPU calculation now uses /proc/stat delta (two reads) (#3, sonic-net#14) - Fix misleading 'critical path estimate' comment (#4) - Fix parallelism timeline comment (60s not 10s) (#5) - Include after-relationship packages in fan stats (#6) - Guard disk I/O division by zero when INTERVAL<=1 (#8) - Remove unused elapsed_line variable (#9) - Remove redundant LIBSWSSCOMMON_DBG check (#10) - Remove active_make_jobs from CSV header comment (#11) - Wire up _RDEPENDS parsing to build reverse deps (#12) - Remove unnecessary 'if v' filter on rdeps JSON (#13) - Remove unused REPORT_FORMAT parameter (sonic-net#15) - Add cycle detection to critical path algorithm (sonic-net#16) - Add execute permission check for companion scripts (sonic-net#17) Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com> --------- Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com> Co-authored-by: Rustiqly <rustiqly@users.noreply.github.com>
Depends on:
croos12/sonic-sairedis#10
croos12/sonic-swss#6
Why I did it
ASAN-instrumented deb packages and Docker images had the same filenames as regular builds, making it impossible to build ASAN on top of a regular build without clobbering and reusing artifacts, such as swss deb and syncd deb that need to be rebuilt for ASAN. Additionally, ENABLE_ASAN was not exported during Docker image builds, so ASAN_OPTIONS were never set in supervisord.conf at runtime.
How I did it
How to verify it
Verify ASAN is active at runtime:
Tested branch (Please provide the tested image version)
Description for the changelog
Fix ASAN build clobbering packages after build on the same system