Fix : gh #130 : Use -O0 not -o0 to set optimisation level for TARGET=linux - #131
Fix : gh #130 : Use -O0 not -o0 to set optimisation level for TARGET=linux#131Ulrond wants to merge 2 commits into
Conversation
-o0 was replaced with -O0 to correctly set optimisation level. This issue only happens with build target LINUX Fixes #130
|
I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
There was a problem hiding this comment.
Pull request overview
Updates the linux-target build defaults in ut-control’s Makefile to use the correct GCC optimization flag (-O0 instead of -o0) when setting the default CC command.
Changes:
- Fixes the linux default compiler flags to use
-O0(optimization level 0) rather than-o0(output file option).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ifeq ($(TARGET),linux) | ||
| CC ?= gcc -ggdb -o0 -Wall | ||
| CC ?= gcc -ggdb -O0 -Wall | ||
| endif |
Adds check_makefile_flags to the tests suite, run before build.sh so a flag typo is reported in seconds rather than after a full build. It walks the build files for a lowercase -o glued to an optimisation level (-o0, -os, -og). gcc accepts these silently and a later -o $@ on the same command line wins, so nothing ever fails - which is why #130 went unnoticed. Comments are stripped before matching, so prose describing the typo does not trip the check. The script also supports asserting what a variable expands to (--expect CC=-ggdb,-O0,-Wall) via make printenv. That is deliberately not wired up yet: 'CC ?=' in the top level Makefile never fires, since GNU make treats ?= as ifeq (origin CC,undefined) and the built-in CC has origin default. The Makefile records this so the expectation can be turned on once that is resolved. Verified by reintroducing -o0: the check fails, exit 1. Refs #130
| check_makefile_flags: | ||
| @$(ECHOE) UT [$@] | ||
| @$(FLAGS_SCRIPT) $(ROOT_DIR)/.. |
| try: | ||
| with open(path, "r", encoding="utf-8", errors="replace") as handle: | ||
| lines = handle.readlines() | ||
| except OSError as error: | ||
| print(f"{RED}Error: cannot read '{path}': {error}{RESET}") | ||
| return None |
Fixes #130
Applies the patch supplied by @looopTools in the issue, with authorship preserved via
git am.-o0is not an optimisation flag — lowercase-ois "write output to this file", so-o0asks for a file named0. TheTARGET=linuxdefault is now-O0.CC ?= gcc -ggdb -O0 -WallTARGET=armis untouched — it takes$(CC)from the SDK and never carried the flag.Note for reviewers: this line is currently a no-op
Worth flagging while the line is under the microscope —
Makefile:117uses?=, which GNU make defines asifeq ($(origin CC),undefined). Make's built-inCC = cchas origindefault, neverundefined, so the assignment never fires:So a standalone
make TARGET=linuxin ut-control compiles with barecc— no-ggdb, no-Wall, no-O0— and when driven from ut-core,CCarrives on the command line and overrides the line regardless. This PR is therefore a correctness fix with no behavioural change on its own.The real
-o0damage came from ut-core passingCC="gcc -ggdb -o0 -Wall"down to this sub-make; that side is fixed in rdkcentral/ut-core#258.If maintainers want the intended defaults to actually apply, the idiom that works while still honouring environment and command-line overrides is:
Deliberately left out of this PR — happy to raise it separately if wanted.
Verification
grep -rn '\-o0' --include=Makefile --include=*.mk --include=*.sh .returns no hits.make TARGET=linuxon this branch builds clean, and no file named0appears in the tree afterwards.Second commit: a regression guard in the tests suite
The typo is two characters, but it survived because no functional test could ever have caught it — gcc accepted
-o0, a later-o $@on the same command line won, and-O0is gcc's default. Every build stayed green. Only a static check on the build files can see it.tests/check_makefile_flags.pyis wired into the suite ascheck_makefile_flagsand runs beforebuild.sh, so a flag typo costs seconds rather than a full build. It walks everyMakefile/*.mk/*.shfor a lowercase-oglued to an optimisation level (-o0,-os,-og,-ofast), skipping third-party trees. Comments are stripped before matching, so prose describing the typo does not trip the check.Verified by reintroducing
-o0on top of the fix:On the fixed tree: 9 build files scanned, 0 findings.
The value assertion is deliberately left off
The script also supports asserting what a variable actually expands to via
make printenv—--expect CC=-ggdb,-O0,-Wall— which is the stronger check, since it catches flags being dropped as well as mistyped. It is not wired up here, because it would fail immediately:CC ?=never fires (see above), soCCexpands to barecc. Turning it on therefore means deciding the?=question first. The reason is recorded in a comment intests/Makefilenext to the target so the next person does not have to rediscover it.The same script is added to ut-core in rdkcentral/ut-core#258, where the value assertion is enabled (
COMPILERandUT_CONTROL_COMPILERmust contain-ggdb -O0 -Wall).