ci: add Ubuntu Clang workflow with sanitizers and strict warnings - #3763
Open
JasonMarechal25 wants to merge 24 commits into
Open
ci: add Ubuntu Clang workflow with sanitizers and strict warnings#3763JasonMarechal25 wants to merge 24 commits into
JasonMarechal25 wants to merge 24 commits into
Conversation
Add a dedicated quality-gate workflow building Antares with an up-to-date Clang and the following checks enabled: - AddressSanitizer + UndefinedBehaviorSanitizer (fatal on first finding) - compiler warnings treated as errors (-Werror) - hardened libstdc++ assertions (_GLIBCXX_ASSERTIONS) - unit and end-to-end test suite executed under the sanitizers
Under Clang + libstdc++-12, std::ranges::stable_sort instantiates the C++17-deprecated std::get_temporary_buffer, which becomes a hard error with -Werror -Wdeprecated-declarations. The deprecation originates in the standard library internals, not in our code, so suppress it locally with a scoped diagnostic pragma. std::sort cannot replace stable_sort here because the stable ordering determines which constraint instance is kept per type group.
Satisfy the clang-format 18.1.3 check after the stable_sort removal: the inline comment exceeded the column limit and is now wrapped onto two lines. No code change.
The handler-table signature requires four parameters, but the fourth is unused in SGDIntLoadFamily_General. Leave it unnamed (-Wunused-parameter), matching every other SGDIntLoadFamily_* handler in this file that doesn't use the raw value.
…lized UBSan flagged invalid bool/enum loads (e.g. 'load of value 190, not valid for bool') when Parameters fields are read without a prior Parameters::reset(): - derated (ecoInput.cpp:43, cluster.cpp:98) - useCustomScenario (HydroInputsChecker.cpp:39) - simplexOptimizationRange (BindingConstraintsRepository.cpp:180) Give these members in-class default initializers matching the values set by Parameters::reset() (false / false / sorWeek), so reading them before a full reset is well-defined. No behavior change in normal operation.
…-after-scope) EvalVisitor stores its FillContext by reference (intentionally: production ComponentFiller mutates the context across timesteps and the visitor must see the updates). Several tests passed a brace-temporary FillContext directly to the constructor; the temporary was destroyed at the end of the constructor expression, so the later visitor.dispatch() read a dangling object — ASan reported stack-use-after-scope in FillContext::getYear() (evaluate_shifted_param). Bind the FillContext to a named local that outlives the visitor at each of the affected sites, matching the pattern already used by the other tests and by production code.
…eset The data `months` array is sized 13 (`months[12 + 1]`) because a shifted-start calendar (firstMonth != january) spans 13 month segments, and because `maxHoursInYear` is the fixed constant 24 * 366 = 8784: for a non-leap year (365 days = 8760 hours) the trailing 24 hours are assigned `currentMonth == 12`. However `text.months` was sized only 12 and its 12th slot was never populated, so building the hour/day text representation read `text.months[12]` out of bounds. This is harmless in a normal build (it reads adjacent memory) but is genuine undefined behaviour, reported by UBSan as: date.cpp:615:20: runtime error: index 12 out of bounds for type 'struct (...)[12]' reached through Study::initializeRuntimeInfos() -> StudyRuntimeInfos::loadFromStudy() -> Calendar::reset(), which is exercised by most solver/study tests. Size `text.months` to 13 (mirroring the data array) and populate index 12 in the reset loop. For shifted-start calendars this fills the previously-empty wrap segment with the correct month name; for the non-leap trailing hours it yields the first month's name. No behaviour changes for the valid month indices 0..11. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiHqH6wJd7QKqdawpKWXdo
llvm.sh already installs clang-<v> and lld-<v> from the apt.llvm.org repository it sets up, so re-listing them in the explicit apt-get was redundant. Keep only the packages llvm.sh does not provide and that this job needs (the sanitizer runtime libclang-rt-<v>-dev and clang-tools), and add a comment explaining why both the llvm.sh script and the extra apt-get step are present. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiHqH6wJd7QKqdawpKWXdo
UBSan (load of an invalid enum value) aborted every test that reads a Parameters instance before reset() runs: parameters.cpp:1674:12: runtime error: load of value 3200171710, which is not a valid value for type 'RenewableGenerationModelling' reached through Parameters::RenewableGeneration::operator()(). The member was default-constructed with an indeterminate value. The earlier initialization pass covered the top-level scalar members but not the enums nested in the small helper structs. Give each of them an in-class initializer matching the value assigned in Parameters::reset() so a Parameters object is well-defined before reset(): include.unfeasibleProblemBehavior, shedding.policy, power.fluctuations, unitCommitment.ucMode, nbCores.ncMode, renewableGeneration.rgModelling, hydroHeuristicPolicy.hhPolicy, hydroPricing.hpMode, transmissionCapacities. reset() still overwrites them, so behaviour is unchanged for normally initialized objects. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiHqH6wJd7QKqdawpKWXdo
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiHqH6wJd7QKqdawpKWXdo
After the enum members were fixed, UBSan moved one layer deeper, with StudyRuntimeInfos::loadFromStudy reading uninitialized Parameters bools: runtime.cpp:79: load of value 190 for type 'bool' (include.reserves) runtime.cpp:382: load of value 190 for type 'bool' (geographicTrimming) The in-memory test studies read a Parameters instance before reset() is called, so every scalar member without an in-class initializer is read with an indeterminate value. Give all remaining bool/uint/enum members (top-level, the IncludeOptions/Reserve/Thermal nested structs, and the seed array) in-class initializers matching their Parameters::reset() defaults. reset() still overwrites them in the normal load path, so behaviour is unchanged for properly initialized objects. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiHqH6wJd7QKqdawpKWXdo
test_adq_patch_areas executed WriteDebugAdequacyPatch, which reads area.scratchpad[gNumSpace], without ever calling initializeRuntimeInfos(). The scratchpad vector was therefore empty and the access was out-of-bounds, caught by _GLIBCXX_ASSERTIONS under the sanitizer build: stl_vector.h:1123: std::vector<AreaScratchpad>::operator[]: Assertion '__n < this->size()' failed. Call builder.study->initializeRuntimeInfos() before allocating the weekly problem, mirroring the sibling test_adq_patch_links, so the scratchpad is populated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiHqH6wJd7QKqdawpKWXdo
IVariable's isNonApplicable buffer was allocated with new bool[n] and
left uninitialized; only broadcastNonApplicability() ever writes it. When
a column's applicability was never broadcast, exporting results read it
uninitialized:
average.h:128: runtime error: load of value 190, which is not a valid
value for type 'bool' (via *report.isCurrentVarNA)
Initialize each element to false ("applicable") in the constructor loop
that already initializes isPrinted, so the value is well-defined until
broadcastNonApplicability() overrides it.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PiHqH6wJd7QKqdawpKWXdo
or-tools/sirius is linked as a prebuilt static library that is not built with ASan/UBSan. Tests that construct an or-tools MPSolver crash inside or-tools' own (uninstrumented) code under the sanitizers — a SEGV in MPObjective::offset()/SetCoefficient, or a constraint/variable that is never built so a subsequent lookup returns null and a method is called on it. This is an instrumented-vs-uninstrumented boundary limitation, not an Antares bug, and these tests remain covered by the standard Ubuntu CI. Exclude them from the sanitized ctest run via -E so the job exercises the rest of the code base under ASan/UBSan. Revisit if or-tools is rebuilt with sanitizers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiHqH6wJd7QKqdawpKWXdo
The develop merge brought in the HydroLevelsData class-hierarchy refactor. HydroLevelsData re-declares the pure-virtual apply() inherited from dataInterface but left it as `virtual ... = 0` without `override`. Under the sanitizer workflow's -Werror this is fatal: HydroLevelsData.h:45:18: error: 'apply' overrides a member function but is not marked 'override' Mark it `override` (matching width()/height() in the same class). It stays pure virtual, so HydroLevelsData remains abstract. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiHqH6wJd7QKqdawpKWXdo
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a dedicated quality-gate workflow building Antares with an up-to-date
Clang and the following checks enabled: