On Sat, Mar 13, 2021 at 08:40:54PM -0800, montytyper@xxxxxxx wrote:
> From: Jordan Montgomery <montytyper@xxxxxxx>
>
> The underlying lexer code generated by flex checks
> if(yychar <= YYEOF) to detect EOF. This returns true
> for any char >= 0x80, resulting in premature EOF and
> causing dtc to generate an invalid FDT.
>
> This patch adds a rule to detect these illegal
> characters and throw a fatal error instead, as well as
> a test case which passes if dtc errors fatally and
> fails otherwise.
>
> Parsing of valid devicetrees should be unaffected by
> this change.
>
> Signed-off-by: Jordan Montgomery <montytyper@xxxxxxx>
This doesn't seem right to me. AIUI, flex although not Unicode-aware,
should be 8-bit clean. If I attempt to build your example file here
without your lexer change, I get just what I'd expect:
$ ../dtc -I dts -O dtb -o /dev/null bad-unicode.dts
Error: bad-unicode.dts:4.30-31 syntax error
FATAL ERROR: Unable to parse input tree
A fatal parse error, because it found a bogus character there, but no
different than if it had been an ASCII bogus character.
> ---
> dtc-lexer.l | 6 ++++++
> tests/bad-unicode.dts | 6 ++++++
> tests/dtc-fatal.sh | 11 +++++++++--
> tests/run_tests.sh | 1 +
> 4 files changed, 22 insertions(+), 2 deletions(-)
> mode change 100644 => 100755 dtc-lexer.l
> create mode 100644 tests/bad-unicode.dts
>
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> old mode 100644
> new mode 100755
> index b3b7270..668875b
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -17,6 +17,7 @@ CHAR_LITERAL '([^']|\\')*'
> WS [[:space:]]
> COMMENT "/*"([^*]|\*+[^*/])*\*+"/"
> LINECOMMENT "//".*\n
> +NONASCII [\x80-\xff]
>
> %{
> #include "dtc.h"
> @@ -245,6 +246,11 @@ static void PRINTF(1, 2) lexical_error(const char *fmt, ...);
> <*>"&&" { return DT_AND; };
> <*>"||" { return DT_OR; };
>
> +<*>{NONASCII} {
> + DPRINT("Illegal character x%X\n", (unsigned)yytext[0]);
> + YY_FATAL_ERROR("Unable to parse non-ASCII character");
> + };
> +
> <*>. {
> DPRINT("Char: %c (\\x%02x)\n", yytext[0],
> (unsigned)yytext[0]);
> diff --git a/tests/bad-unicode.dts b/tests/bad-unicode.dts
> new file mode 100644
> index 0000000..19b3d2f
> --- /dev/null
> +++ b/tests/bad-unicode.dts
> @@ -0,0 +1,6 @@
> +/dts-v1/;
> +
> +/ {
> + prop-str = "hello world";⌘
> + prop-str2 = "hello world";
> +};
> diff --git a/tests/dtc-fatal.sh b/tests/dtc-fatal.sh
> index 08a4d29..56cdc1c 100755
> --- a/tests/dtc-fatal.sh
> +++ b/tests/dtc-fatal.sh
> @@ -3,13 +3,20 @@
> SRCDIR=`dirname "$0"`
> . "$SRCDIR/testutils.sh"
>
> +if [ "$1" = "-r" ]; then
> + expected="$2"
> + shift 2
> +else
> + expected="1"
> +fi
> +
> verbose_run $VALGRIND "$DTC" -o/dev/null "$@"
> ret="$?"
>
> if [ "$ret" -gt 127 ]; then
> FAIL "dtc killed by signal (ret=$ret)"
> -elif [ "$ret" != "1" ]; then
> - FAIL "dtc returned incorrect status $ret instead of 1"
> +elif [ "$ret" != "$expected" ]; then
> + FAIL "dtc returned incorrect status $ret instead of $expected"
> fi
>
> PASS
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 4b8dada..c9bc095 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -764,6 +764,7 @@ dtc_tests () {
> run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb nosuchfile.dts
> run_sh_test "$SRCDIR/dtc-fatal.sh" -I dtb -O dtb nosuchfile.dtb
> run_sh_test "$SRCDIR/dtc-fatal.sh" -I fs -O dtb nosuchfile
> + run_sh_test "$SRCDIR/dtc-fatal.sh" -r 2 -I dts -O dtb bad-unicode.dts
>
> # Dependencies
> run_dtc_test -I dts -O dtb -o dependencies.test.dtb -d dependencies.test.d "$SRCDIR/dependencies.dts"
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
