From dbd1c03aa3f1077953883b2eed226ec259653b2c Mon Sep 17 00:00:00 2001 From: tzuohann <2057796+tzuohann@users.noreply.github.com> Date: Fri, 18 Sep 2026 12:41:07 -0400 Subject: [PATCH] halcompile: reject declarations that claim the same HAL name A name declared in a .comp file is exported under a mangled HAL identifier: underscores become dashes and a trailing dash or period is removed (comp.adoc, HALNAME). check_name_ok() compares only declared names, so a component declaring both x_y and x_y_ exported both as x-y. halcompile accepted it and the module failed at loadrt: HAL: ERROR: duplicate pin 'collide.0.x-y' collide: rtapi_app_main: Invalid argument (-22) A declaration does not claim one name, so comparing mangled declared names is not enough: - an array claims one name per element, and to_hal() turns the "#" block into a printf conversion, so 'x_#[4]' and 'x_0' both claim x-0 while their mangled forms never compare equal; - hal_export_funct() creates .time, .tmax and .tmax-increased in the pin and param namespace, so 'function f' and 'pin f.time' collide although only one of them is written down. Those are claimed in the last rule of the grammar rather than at the declaration, since a userspace component exports no function and 'option userspace' may follow it. claim() therefore records the set of HAL names each declaration takes. Pins and params share one set, because hal_lib.c refuses a pin whose name is already a param and the reverse; functions have their own. An 'if' condition does not exempt a declaration: telling two conditions apart would mean evaluating the C, and overlapping personality pins are a design error anyway. Arrays are limited to 256 elements. The largest in the tree is 32. Two shapes master accepts are now refused: an array over 256 elements, and two declarations that claim one name under mutually exclusive 'if' conditions. Docs: NAMES section in the halcompile man page, linking to the HALNAME rules in comp.adoc, which gains a [[sec:halname]] anchor to link at. Tests: tests/halcompile/halname covers pin/pin, pin/param, function/function, array/scalar, function/derived-pin and personality collisions and the array limit, and asserts that a pin and a function mangling to one name are accepted, since their namespaces differ. All 146 .comp files on master preprocess to the same C, and with identical exit status, as master. Co-Authored-By: Claude Opus 5 (1M context) --- docs/src/hal/comp.adoc | 15 ++++- docs/src/man/man1/halcompile.1.adoc | 15 +++++ src/hal/utils/halcompile.g | 64 +++++++++++++++++-- tests/halcompile/halname/.gitignore | 8 +++ tests/halcompile/halname/array_limit.comp | 6 ++ tests/halcompile/halname/collide_array.comp | 7 ++ .../halname/collide_funct_time.comp | 6 ++ .../halcompile/halname/collide_function.comp | 8 +++ .../halname/collide_personality.comp | 7 ++ .../halcompile/halname/collide_pin_param.comp | 7 ++ tests/halcompile/halname/collide_pin_pin.comp | 7 ++ tests/halcompile/halname/expected | 21 ++++++ .../halname/separate_namespaces.comp | 6 ++ tests/halcompile/halname/test.sh | 20 ++++++ 14 files changed, 191 insertions(+), 6 deletions(-) create mode 100644 tests/halcompile/halname/.gitignore create mode 100644 tests/halcompile/halname/array_limit.comp create mode 100644 tests/halcompile/halname/collide_array.comp create mode 100644 tests/halcompile/halname/collide_funct_time.comp create mode 100644 tests/halcompile/halname/collide_function.comp create mode 100644 tests/halcompile/halname/collide_personality.comp create mode 100644 tests/halcompile/halname/collide_pin_param.comp create mode 100644 tests/halcompile/halname/collide_pin_pin.comp create mode 100644 tests/halcompile/halname/expected create mode 100644 tests/halcompile/halname/separate_namespaces.comp create mode 100755 tests/halcompile/halname/test.sh diff --git a/docs/src/hal/comp.adoc b/docs/src/hal/comp.adoc index aa6d5816bb5..c4b308b3494 100644 --- a/docs/src/hal/comp.adoc +++ b/docs/src/hal/comp.adoc @@ -217,6 +217,7 @@ When used to create a C identifier, the following changes are applied to the HAL A trailing "_" is retained, so that HAL identifiers which would otherwise collide with reserved names or keywords (e.g., 'min') can be used. +[[sec:halname]] [width="90%",options="header"] |=== |HALNAME | C Identifier | HAL Identifier @@ -227,11 +228,21 @@ A trailing "_" is retained, so that HAL identifiers which would otherwise collid |x.## | x(MM) | x.MM |=== +[NOTE] +Two declarations that claim the same HAL identifier -- 'x_y_z' and 'x_y_z_' in +the table above -- are rejected by `halcompile`; they would otherwise be +refused by HAL at `loadrt`. +An array claims one identifier per element, so 'x_#' with '[4]' and 'x_0' +collide, and a function claims '.time', '.tmax' and +'.tmax-increased' as well, since those are created alongside it. +Pins and parameters share one namespace; functions have their own. +An 'if' condition does not exempt a declaration. + * 'if CONDITION' - An expression involving the variable 'personality' which is nonzero when the pin or parameter should be created. -* 'SIZE' - A number that gives the size of an array. The array items are numbered from 0 to 'SIZE'-1. +* 'SIZE' - A number that gives the size of an array, at most 256. The array items are numbered from 0 to 'SIZE'-1. -* 'MAXSIZE : CONDSIZE' - A number that gives the maximum size of the array, +* 'MAXSIZE : CONDSIZE' - A number that gives the maximum size of the array, at most 256, followed by an expression involving the variable 'personality' and which always evaluates to less than 'MAXSIZE'. When the array is created its size will be 'CONDSIZE'. diff --git a/docs/src/man/man1/halcompile.1.adoc b/docs/src/man/man1/halcompile.1.adoc index c06cd1b8688..593be844b17 100644 --- a/docs/src/man/man1/halcompile.1.adoc +++ b/docs/src/man/man1/halcompile.1.adoc @@ -97,6 +97,21 @@ Extra arguments passed to the linker. require _sudo_ to write to system directories. * Preprocess *.comp* files into *.c* files (the *--preprocess* flag) +== NAMES + +A name declared in a *.comp* file is exported under a mangled HAL identifier: +underscores become dashes, and a trailing dash or period is removed. +*pin in real my_input* is reached from HAL as *component.N.my-input*, and +*loadrt my_comp* exports its pins under *my-comp.N.*. + +Two declarations that claim the same HAL name are rejected, counting every +element of an array and the *.time*, *.tmax* and *.tmax-increased* names a +function brings with it. Pins and parameters share one namespace; functions +have their own, so a pin and a function may mangle to the same name. + +Full rules: HALNAME under _Syntax_ in the _HAL Component Generator_ +documentation, https://linuxcnc.org/docs/html/hal/comp.html#sec:halname + == SEE ALSO * _Halcompile_ / _HAL Component Generator_ in the LinuxCNC documentation for a diff --git a/src/hal/utils/halcompile.g b/src/hal/utils/halcompile.g index 2690dc61df7..59f6f729bb3 100644 --- a/src/hal/utils/halcompile.g +++ b/src/hal/utils/halcompile.g @@ -35,7 +35,7 @@ parser Hal: token POP: "[-()+*/:?]|&&|\\|\\||personality|==|&|!=|<<|<|<=|>>|>|>=" token TSTRING: "r?\"\"\"(\\.|\\\n|[^\\\"]|\"(?!\"\")|\n)*\"\"\"" - rule File: ComponentDeclaration Declaration* "$" {{ return True }} + rule File: ComponentDeclaration Declaration* "$" {{ end_of_declarations(); return True }} rule ComponentDeclaration: "component" NAME OptString";" {{ comp(NAME, OptString); }} rule Declaration: @@ -113,6 +113,9 @@ MAX_USERSPACE_NAMES = 16 # for userspace (loadusr) components # exported is computed modulo MAX_PERSONALITIES MAX_PERSONALITIES = 64 +# An array larger than this is almost certainly a mistake. +MAX_ARRAY_SIZE = 256 + mp_decl_map = {'int': 'RTAPI_MP_INT', 'dummy': None} # These are symbols that comp puts in the global namespace of the C file it @@ -157,23 +160,27 @@ newtypes = ['bool', 'sint', 'uint', 'si32', 'ui32', 'real'] def initialize(): global functions, params, pins, comp_name, names, docs, variables - global modparams, includes + global modparams, includes, hal_pin_names, hal_funct_names + global funct_derived_claims functions = []; params = []; pins = []; options = {}; variables = [] modparams = []; docs = []; includes = []; comp_name = None names = {} + hal_pin_names = {} + hal_funct_names = {} + funct_derived_claims = [] def Warn(msg, *args): if args: msg = msg % args print("%s:%d: Warning: %s" % (S.filename, S.line, msg), file=sys.stderr) -def Error(msg, *args): +def Error(msg, *args, pos=None): if args: msg = msg % args - raise runtime.SyntaxError(S.get_pos(), msg, None) + raise runtime.SyntaxError(pos or S.get_pos(), msg, None) def optfp_warn(state): s = "nofp" if 0 == state else "fp" @@ -216,6 +223,10 @@ def checkarray(name, array): if array: if hashes == 0: Error("Array name contains no #: %r" % name) if hashes > 1: Error("Array name contains more than one block of #: %r" % name) + size = array_size(array) + if size > MAX_ARRAY_SIZE: + Error("Array size %d exceeds the maximum of %d: %r" + % (size, MAX_ARRAY_SIZE, name)) else: if hashes > 0: Error("Non-array name contains #: %r" % name) @@ -225,10 +236,46 @@ def check_name_ok(name): if name in names: Error("Duplicate item name %s" % name) +# [MAXSIZE : CONDSIZE] varies the count with personality; MAXSIZE bounds it. +def array_size(array): + return array[0] if isinstance(array, tuple) else array + +# Every HAL name a declaration claims. to_hal() turns the "#" block into a +# printf conversion, so an array claims one name per element: 'x_#[4]' claims +# x-0..x-3 and collides with 'x_0'. +def hal_names_of(name, array): + hal_name = to_hal(name) + if not array: + return [hal_name] + return [hal_name % j for j in range(array_size(array))] + +# check_name_ok() compares declared names only, so two declarations mangling to +# one HAL name compiled and then failed at loadrt. An 'if' condition does not +# exempt a declaration: telling two conditions apart would mean evaluating C. +def claim(seen, what, hal_names, pos=None): + for hal_name in hal_names: + if hal_name in seen: + shown = "'%s'" % hal_name if hal_name else "the instance name" + Error("Name collision: %s and %s both become %s and are " + "indistinguishable." % (seen[hal_name], what, shown), pos=pos) + seen[hal_name] = what + +# hal_export_funct() also creates .time, .tmax and .tmax-increased in +# the pin and param namespace, but only for a realtime component, and +# 'option userspace' may follow the function. So these wait for the last rule +# of the grammar, where the whole file has been seen and an Error() is still +# reported the way every other one is. +def end_of_declarations(): + if options.get("userspace"): + return + for pos, what, hal_name in funct_derived_claims: + claim(hal_pin_names, what, [hal_name], pos) + def pin(name, type_, array, dir_, doc, value, personality): checkarray(name, array) type_ = type2type(type_) check_name_ok(name) + claim(hal_pin_names, "pin '%s'" % name, hal_names_of(name, array)) docs.append(('pin', name, type_, array, dir_, doc, value, personality)) names[name] = None pins.append((name, type_, array, dir_, value, personality)) @@ -237,12 +284,21 @@ def param(name, type_, array, dir_, doc, value, personality): checkarray(name, array) type_ = type2type(type_) check_name_ok(name) + # one namespace with pins in hal_lib.c, so a pin and a param collide + claim(hal_pin_names, "param '%s'" % name, hal_names_of(name, array)) docs.append(('param', name, type_, array, dir_, doc, value, personality)) names[name] = None params.append((name, type_, array, dir_, value, personality)) def function(name, fp, doc): check_name_ok(name) + hal_name = to_hal(name) + claim(hal_funct_names, "function '%s'" % name, [hal_name]) + pos = S.get_pos() + for suffix in ("time", "tmax", "tmax-increased"): + funct_derived_claims.append( + (pos, "the .%s of function '%s'" % (suffix, name), + hal_name + "." + suffix if hal_name else suffix)) docs.append(('funct', name, fp, doc)) names[name] = None functions.append((name, fp)) diff --git a/tests/halcompile/halname/.gitignore b/tests/halcompile/halname/.gitignore new file mode 100644 index 00000000000..93c030198ab --- /dev/null +++ b/tests/halcompile/halname/.gitignore @@ -0,0 +1,8 @@ +array_limit.c +collide_array.c +collide_funct_time.c +collide_function.c +collide_personality.c +collide_pin_param.c +collide_pin_pin.c +separate_namespaces.c diff --git a/tests/halcompile/halname/array_limit.comp b/tests/halcompile/halname/array_limit.comp new file mode 100644 index 00000000000..1738fd8da38 --- /dev/null +++ b/tests/halcompile/halname/array_limit.comp @@ -0,0 +1,6 @@ +component array_limit; +license "GPL"; +pin in bool x_#[257]; +function _; +;; +FUNCTION(_) {} diff --git a/tests/halcompile/halname/collide_array.comp b/tests/halcompile/halname/collide_array.comp new file mode 100644 index 00000000000..eb33bbfcd5a --- /dev/null +++ b/tests/halcompile/halname/collide_array.comp @@ -0,0 +1,7 @@ +component collide_array; +license "GPL"; +pin in bool x_#[4]; +pin out bool x_0; +function _; +;; +FUNCTION(_) {} diff --git a/tests/halcompile/halname/collide_funct_time.comp b/tests/halcompile/halname/collide_funct_time.comp new file mode 100644 index 00000000000..3c890f79d74 --- /dev/null +++ b/tests/halcompile/halname/collide_funct_time.comp @@ -0,0 +1,6 @@ +component collide_funct_time; +license "GPL"; +pin out sint f.time; +function f; +;; +FUNCTION(f) {} diff --git a/tests/halcompile/halname/collide_function.comp b/tests/halcompile/halname/collide_function.comp new file mode 100644 index 00000000000..d18185629aa --- /dev/null +++ b/tests/halcompile/halname/collide_function.comp @@ -0,0 +1,8 @@ +component collide_function; +license "GPL"; +pin in bool enable; +function a_b; +function a_b_; +;; +FUNCTION(a_b) {} +FUNCTION(a_b_) {} diff --git a/tests/halcompile/halname/collide_personality.comp b/tests/halcompile/halname/collide_personality.comp new file mode 100644 index 00000000000..715bd41f951 --- /dev/null +++ b/tests/halcompile/halname/collide_personality.comp @@ -0,0 +1,7 @@ +component collide_personality; +license "GPL"; +pin in bool x_y if personality == 1; +pin out bool x_y_ if personality == 0; +function _; +;; +FUNCTION(_) {} diff --git a/tests/halcompile/halname/collide_pin_param.comp b/tests/halcompile/halname/collide_pin_param.comp new file mode 100644 index 00000000000..77076b21568 --- /dev/null +++ b/tests/halcompile/halname/collide_pin_param.comp @@ -0,0 +1,7 @@ +component collide_pin_param; +license "GPL"; +pin in bool x_y; +param rw bool x_y_; +function _; +;; +FUNCTION(_) {} diff --git a/tests/halcompile/halname/collide_pin_pin.comp b/tests/halcompile/halname/collide_pin_pin.comp new file mode 100644 index 00000000000..fd02b13add3 --- /dev/null +++ b/tests/halcompile/halname/collide_pin_pin.comp @@ -0,0 +1,7 @@ +component collide_pin_pin; +license "GPL"; +pin in bool x_y; +pin out bool x_y_; +function _; +;; +FUNCTION(_) {} diff --git a/tests/halcompile/halname/expected b/tests/halcompile/halname/expected new file mode 100644 index 00000000000..2e1cb5e9e3b --- /dev/null +++ b/tests/halcompile/halname/expected @@ -0,0 +1,21 @@ +collide_pin_pin.comp:4:19: Name collision: pin 'x_y' and pin 'x_y_' both become 'x-y' and are indistinguishable. +> pin out bool x_y_; +> ^ +collide_pin_param.comp:4:20: Name collision: pin 'x_y' and param 'x_y_' both become 'x-y' and are indistinguishable. +> param rw bool x_y_; +> ^ +collide_function.comp:5:15: Name collision: function 'a_b' and function 'a_b_' both become 'a-b' and are indistinguishable. +> function a_b_; +> ^ +collide_array.comp:4:18: Name collision: pin 'x_#' and pin 'x_0' both become 'x-0' and are indistinguishable. +> pin out bool x_0; +> ^ +collide_funct_time.comp:4:12: Name collision: pin 'f.time' and the .time of function 'f' both become 'f.time' and are indistinguishable. +> function f; +> ^ +collide_personality.comp:4:39: Name collision: pin 'x_y' and pin 'x_y_' both become 'x-y' and are indistinguishable. +> pin out bool x_y_ if personality == 0; +> ^ +array_limit.comp:3:22: Array size 257 exceeds the maximum of 256: 'x_#' +> pin in bool x_#[257]; +> ^ diff --git a/tests/halcompile/halname/separate_namespaces.comp b/tests/halcompile/halname/separate_namespaces.comp new file mode 100644 index 00000000000..4de56205d00 --- /dev/null +++ b/tests/halcompile/halname/separate_namespaces.comp @@ -0,0 +1,6 @@ +component separate_namespaces; +license "GPL"; +pin in bool x_y; +function x_y_; +;; +FUNCTION(x_y_) {} diff --git a/tests/halcompile/halname/test.sh b/tests/halcompile/halname/test.sh new file mode 100755 index 00000000000..a293f0926de --- /dev/null +++ b/tests/halcompile/halname/test.sh @@ -0,0 +1,20 @@ +#!/bin/bash + +# Two declarations that claim one HAL name must be rejected here, not left to +# fail at loadrt as "HAL: ERROR: duplicate pin". Pins and params share one +# namespace in hal_lib.c; functions have their own, but hal_export_funct() +# also creates .time, .tmax and .tmax-increased as a pin and params. +# An array claims one name per element, and is limited to 256 elements. +for c in collide_pin_pin collide_pin_param collide_function collide_array \ + collide_funct_time collide_personality array_limit; do + rm -f "$c.c" + halcompile --preprocess "$c.comp" 2>&1 && echo "halcompile accepted $c.comp" + [ -f "$c.c" ] && echo "halcompile produced $c.c" +done + +# A pin and a function that mangle to one name are exported into different +# HAL namespaces, so they do not collide. +c=separate_namespaces +rm -f "$c.c" +halcompile --preprocess "$c.comp" 2>&1 || echo "halcompile rejected $c.comp" +[ -f "$c.c" ] || echo "halcompile did not produce $c.c"