On 21/07/18 07:55, Luc Van Oostenryck wrote:
> PSEUDO_VALs have no associated signedness (and more generally, no
> type associated with them). This is generally not a problem but
> in some circonstances it is.
s/circonstances/circumstances/
>
> Add a description of the problem and a testcase.
>
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx>
> ---
> Documentation/Problems.txt | 32 ++++++++++++++++++++++++++++++++
> validation/shift-negative.c | 18 ++++++++++++++++++
> 2 files changed, 50 insertions(+)
> create mode 100644 Documentation/Problems.txt
> create mode 100644 validation/shift-negative.c
>
> diff --git a/Documentation/Problems.txt b/Documentation/Problems.txt
> new file mode 100644
> index 000000000..f9fbbd28c
> --- /dev/null
> +++ b/Documentation/Problems.txt
> @@ -0,0 +1,32 @@
> +signedness (and type) of PSEUDO_VAL
> +-----------------------------------
> +It is currently impossible, after linearization, to determine
> +if a constant is negative or not.
> +
> +Before linearization, these values are handled as expressions
> +which always have a type associated with them. It is thus easy
> +to get the corresponding size & signedness.
> +
> +This is not true anymore after linearization. PSEUDO_VALs are
> +created from EXPR_VALs without any conversion (sign-extension)
> +and have no type directly attached to them. If a type is needed
> +it must be retrieved from some context, like the type of the
> +instruction using them. This is fine for most instructions,
> +for example OP_ADD, where the type of both operands and of
> +the result must all be the same but this is not the case for:
> +* comparison instructions:
> + both operands should have the same type but the type
> + of the result is independent of the operands' type.
Huh? The sign of the result is irrelevant right? (ie its's a
boolean type, for which sign has no meaning.)
> +* shift instructions:
> + the type of the result must be the same as the type
> + of the left operand but the type of the right operand
> + is independent.
But for constant shifts, the shift direction can be flipped
and the shift amount made non-negative, right?
> +
> +
> +A possible solution for the signedness would be, during
> +linearization and simplification, to keep values fully sign-
> +extended (currently, '(int) -1' is stored as 0x00000000ffffffff,
> +not 0xffffffffffffffff) but the consequences must be investigated.
> +
> +Another solution would be to add to struct instruction a field for
> +the type of these now-untyped operands (like 'orig_type' for casts).
seems reasonable.
ATB,
Ramsay Jones
> diff --git a/validation/shift-negative.c b/validation/shift-negative.c
> new file mode 100644
> index 000000000..033a51a85
> --- /dev/null
> +++ b/validation/shift-negative.c
> @@ -0,0 +1,18 @@
> +unsigned char fn1(unsigned char a) { return a >> -1; }
> +unsigned char fn2(unsigned char a) { return a >> ~0; }
> +
> +unsigned char fo1(unsigned char a) { return a >> ((a & 0) | -1); }
> +unsigned char fo2(unsigned char a) { return a >> ((a & 0) ^ ~0); }
> +
> +/*
> + * check-name: shift-negative
> + * check-command: sparse -Wno-decl $file
> + * check-known-to-fail
> + *
> + * check-error-start
> +shift-negative.c:1:47: warning: shift is negative (-1)
> +shift-negative.c:2:47: warning: shift is negative (-1)
> +shift-negative.c:4:61: warning: shift is negative (-1)
> +shift-negative.c:5:61: warning: shift is negative (-1)
> + * check-error-end
> + */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
