On 12.05.2023 14:45, Andrew Cooper wrote:
> When adding new featureset words, it is convenient to split the work into
> several patches. However, GCC 12 spotted that the way we prefer to split the
> work results in a real (transient) breakage whereby the policy <-> featureset
> helpers perform out-of-bounds accesses on the featureset array.
>
> Fix this by having gen-cpuid.py calculate FEATURESET_NR_ENTRIES from the
> comments describing the word blocks, rather than from the XEN_CPUFEATURE()
> with the greatest value.
>
> For simplicty, require that the word blocks appear in order. This can be
> revisted if we find a good reason to have blocks out of order.
>
> No functional change.
>
> Reported-by: Jan Beulich <jbeul...@suse.com>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
As far as my Python goes:
Reviewed-by: Jan Beulich <jbeul...@suse.com>
Just one remark further down.
> This supercedes the entire "x86: Fix transient build breakage with featureset
> additions" series, but doesn't really feel as if it ought to be labelled v2
Thank you for re-doing this altogether. I think it's more safe this way,
and it now being less intrusive is imo also beneficial.
> --- a/xen/tools/gen-cpuid.py
> +++ b/xen/tools/gen-cpuid.py
> @@ -50,13 +50,37 @@ def parse_definitions(state):
> "\s+([\s\d]+\*[\s\d]+\+[\s\d]+)\)"
> "\s+/\*([\w!]*) .*$")
>
> + word_regex = re.compile(
> + r"^/\* .* word (\d*) \*/$")
> + last_word = -1
> +
> this = sys.modules[__name__]
>
> for l in state.input.readlines():
> - # Short circuit the regex...
> - if not l.startswith("XEN_CPUFEATURE("):
> +
> + # Short circuit the regexes...
> + if not (l.startswith("XEN_CPUFEATURE(") or
> + l.startswith("/* ")):
> continue
>
> + # Handle /* ... word $N */ lines
> + if l.startswith("/* "):
> +
> + res = word_regex.match(l)
> + if res is None:
> + continue # Some other comment
> +
> + word = int(res.groups()[0])
> +
> + if word != last_word + 1:
> + raise Fail("Featureset word %u out of order (last word %u)"
> + % (word, last_word))
> +
> + last_word = word
> + state.nr_entries = word + 1
> + continue
> +
> + # Handle XEN_CPUFEATURE( lines
> res = feat_regex.match(l)
>
> if res is None:
> @@ -94,6 +118,15 @@ def parse_definitions(state):
> if len(state.names) == 0:
> raise Fail("No features found")
>
> + if state.nr_entries == 0:
> + raise Fail("No featureset word info found")
> +
> + max_val = max(state.names.keys())
> + if (max_val >> 5) + 1 > state.nr_entries:
Maybe
if (max_val >> 5) >= state.nr_entries:
?
Jan
