From fa63795f40875541e05b90970825bb57132fcc3d Mon Sep 17 00:00:00 2001 From: Alex Coplan Date: Mon, 10 Aug 2020 17:44:02 +0100 Subject: [PATCH] aarch64: Don't assert on long sysreg names This patch fixes an assertion failure on long system register operands in the AArch64 backend. See the new testcase for an input which reproduces the issue. gas/ChangeLog: * config/tc-aarch64.c (parse_sys_reg): Don't assert when parsing a long system register. (parse_sys_ins_reg): Likewise. (sysreg_hash_insert): New. (md_begin): Use sysreg_hash_insert() to ensure all system registers are no longer than the maximum length at startup. * testsuite/gas/aarch64/invalid-sysreg-assert.d: New test. * testsuite/gas/aarch64/invalid-sysreg-assert.l: Error output. * testsuite/gas/aarch64/invalid-sysreg-assert.s: Input. include/ChangeLog: * opcode/aarch64.h (AARCH64_MAX_SYSREG_NAME_LEN): New. --- gas/ChangeLog | 12 ++++++ gas/config/tc-aarch64.c | 43 +++++++++++++------ .../gas/aarch64/invalid-sysreg-assert.d | 3 ++ .../gas/aarch64/invalid-sysreg-assert.l | 2 + .../gas/aarch64/invalid-sysreg-assert.s | 2 + include/ChangeLog | 4 ++ include/opcode/aarch64.h | 2 + 7 files changed, 55 insertions(+), 13 deletions(-) create mode 100644 gas/testsuite/gas/aarch64/invalid-sysreg-assert.d create mode 100644 gas/testsuite/gas/aarch64/invalid-sysreg-assert.l create mode 100644 gas/testsuite/gas/aarch64/invalid-sysreg-assert.s diff --git a/gas/ChangeLog b/gas/ChangeLog index 03628a685d0..07def54083e 100644 --- a/gas/ChangeLog +++ b/gas/ChangeLog @@ -1,3 +1,15 @@ +2020-08-10 Alex Coplan + + * config/tc-aarch64.c (parse_sys_reg): Don't assert when parsing + a long system register. + (parse_sys_ins_reg): Likewise. + (sysreg_hash_insert): New. + (md_begin): Use sysreg_hash_insert() to ensure all system + registers are no longer than the maximum length at startup. + * testsuite/gas/aarch64/invalid-sysreg-assert.d: New test. + * testsuite/gas/aarch64/invalid-sysreg-assert.l: Error output. + * testsuite/gas/aarch64/invalid-sysreg-assert.s: Input. + 2020-08-10 Przemyslaw Wirkus * config/tc-aarch64.c (parse_sys_reg): Call to diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c index a46a474af56..fdac91ee9d3 100644 --- a/gas/config/tc-aarch64.c +++ b/gas/config/tc-aarch64.c @@ -4100,17 +4100,21 @@ parse_sys_reg (char **str, struct hash_control *sys_regs, uint32_t* flags) { char *p, *q; - char buf[32]; + char buf[AARCH64_MAX_SYSREG_NAME_LEN]; const aarch64_sys_reg *o; int value; p = buf; for (q = *str; ISALNUM (*q) || *q == '_'; q++) - if (p < buf + 31) + if (p < buf + (sizeof (buf) - 1)) *p++ = TOLOWER (*q); *p = '\0'; - /* Assert that BUF be large enough. */ - gas_assert (p - buf == q - *str); + + /* If the name is longer than AARCH64_MAX_SYSREG_NAME_LEN then it cannot be a + valid system register. This is enforced by construction of the hash + table. */ + if (p - buf != q - *str) + return PARSE_FAIL; o = hash_find (sys_regs, buf); if (!o) @@ -4161,15 +4165,21 @@ static const aarch64_sys_ins_reg * parse_sys_ins_reg (char **str, struct hash_control *sys_ins_regs) { char *p, *q; - char buf[32]; + char buf[AARCH64_MAX_SYSREG_NAME_LEN]; const aarch64_sys_ins_reg *o; p = buf; for (q = *str; ISALNUM (*q) || *q == '_'; q++) - if (p < buf + 31) + if (p < buf + (sizeof (buf) - 1)) *p++ = TOLOWER (*q); *p = '\0'; + /* If the name is longer than AARCH64_MAX_SYSREG_NAME_LEN then it cannot be a + valid system register. This is enforced by construction of the hash + table. */ + if (p - buf != q - *str) + return NULL; + o = hash_find (sys_ins_regs, buf); if (!o) return NULL; @@ -8620,6 +8630,13 @@ checked_hash_insert (struct hash_control *table, const char *key, void *value) printf ("Internal Error: Can't hash %s\n", key); } +static void +sysreg_hash_insert (struct hash_control *table, const char *key, void *value) +{ + gas_assert (strlen (key) < AARCH64_MAX_SYSREG_NAME_LEN); + checked_hash_insert (table, key, value); +} + static void fill_instruction_hash_table (void) { @@ -8694,36 +8711,36 @@ md_begin (void) fill_instruction_hash_table (); for (i = 0; aarch64_sys_regs[i].name != NULL; ++i) - checked_hash_insert (aarch64_sys_regs_hsh, aarch64_sys_regs[i].name, + sysreg_hash_insert (aarch64_sys_regs_hsh, aarch64_sys_regs[i].name, (void *) (aarch64_sys_regs + i)); for (i = 0; aarch64_pstatefields[i].name != NULL; ++i) - checked_hash_insert (aarch64_pstatefield_hsh, + sysreg_hash_insert (aarch64_pstatefield_hsh, aarch64_pstatefields[i].name, (void *) (aarch64_pstatefields + i)); for (i = 0; aarch64_sys_regs_ic[i].name != NULL; i++) - checked_hash_insert (aarch64_sys_regs_ic_hsh, + sysreg_hash_insert (aarch64_sys_regs_ic_hsh, aarch64_sys_regs_ic[i].name, (void *) (aarch64_sys_regs_ic + i)); for (i = 0; aarch64_sys_regs_dc[i].name != NULL; i++) - checked_hash_insert (aarch64_sys_regs_dc_hsh, + sysreg_hash_insert (aarch64_sys_regs_dc_hsh, aarch64_sys_regs_dc[i].name, (void *) (aarch64_sys_regs_dc + i)); for (i = 0; aarch64_sys_regs_at[i].name != NULL; i++) - checked_hash_insert (aarch64_sys_regs_at_hsh, + sysreg_hash_insert (aarch64_sys_regs_at_hsh, aarch64_sys_regs_at[i].name, (void *) (aarch64_sys_regs_at + i)); for (i = 0; aarch64_sys_regs_tlbi[i].name != NULL; i++) - checked_hash_insert (aarch64_sys_regs_tlbi_hsh, + sysreg_hash_insert (aarch64_sys_regs_tlbi_hsh, aarch64_sys_regs_tlbi[i].name, (void *) (aarch64_sys_regs_tlbi + i)); for (i = 0; aarch64_sys_regs_sr[i].name != NULL; i++) - checked_hash_insert (aarch64_sys_regs_sr_hsh, + sysreg_hash_insert (aarch64_sys_regs_sr_hsh, aarch64_sys_regs_sr[i].name, (void *) (aarch64_sys_regs_sr + i)); diff --git a/gas/testsuite/gas/aarch64/invalid-sysreg-assert.d b/gas/testsuite/gas/aarch64/invalid-sysreg-assert.d new file mode 100644 index 00000000000..a6279bbe7c2 --- /dev/null +++ b/gas/testsuite/gas/aarch64/invalid-sysreg-assert.d @@ -0,0 +1,3 @@ +#name: don't assert on long system register +#source: invalid-sysreg-assert.s +#error_output: invalid-sysreg-assert.l diff --git a/gas/testsuite/gas/aarch64/invalid-sysreg-assert.l b/gas/testsuite/gas/aarch64/invalid-sysreg-assert.l new file mode 100644 index 00000000000..b6049108b07 --- /dev/null +++ b/gas/testsuite/gas/aarch64/invalid-sysreg-assert.l @@ -0,0 +1,2 @@ +[^:]*: Assembler messages: +.*: Error: unknown or missing system register name at operand 1 -- `msr 00000000000000000000000000000000' diff --git a/gas/testsuite/gas/aarch64/invalid-sysreg-assert.s b/gas/testsuite/gas/aarch64/invalid-sysreg-assert.s new file mode 100644 index 00000000000..8b3706fd9cd --- /dev/null +++ b/gas/testsuite/gas/aarch64/invalid-sysreg-assert.s @@ -0,0 +1,2 @@ +// This input caused an assertion failure in parse_sys_reg. +msr 00000000000000000000000000000000 diff --git a/include/ChangeLog b/include/ChangeLog index 27f14cc111e..d0475de1ce9 100644 --- a/include/ChangeLog +++ b/include/ChangeLog @@ -1,3 +1,7 @@ +2020-08-10 Alex Coplan + + * opcode/aarch64.h (AARCH64_MAX_SYSREG_NAME_LEN): New. + 2020-08-10 Przemyslaw Wirkus * opcode/aarch64.h (aarch64_sys_reg_deprecated_p): Functions diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h index 4b71f93b946..5122ea85658 100644 --- a/include/opcode/aarch64.h +++ b/include/opcode/aarch64.h @@ -943,6 +943,8 @@ extern const struct aarch64_name_value_pair aarch64_barrier_options [16]; extern const struct aarch64_name_value_pair aarch64_prfops [32]; extern const struct aarch64_name_value_pair aarch64_hint_options []; +#define AARCH64_MAX_SYSREG_NAME_LEN 32 + typedef struct { const char * name; -- 2.30.2