[PATCH] Riscv ld-elf/stab failure, and fake label cleanup.
Palmer Dabbelt
palmer@sifive.com
Tue Nov 21 15:56:00 GMT 2017
On Mon, 20 Nov 2017 14:41:04 PST (-0800), Jim Wilson wrote:
> This is an attempt to fix a riscv ld-elf/stab testsuite failure. This is
> failing because the riscv port redefines FAKE_LABEL_NAME, and there turns out
> to be a lot of hard coded assumptions about the value of FAKE_LABEL_NAME in
> gas.
For the record (Jim knows this, he explained half of it to me :)), we want it
this way because, due to all our linker relaxation, we frequently end up with
FAKE_LABEL_NAME in object files. Users kept thinking the '\001' in their
objdump output was a bug, so we changed it.
> The first part of this patch involves changing code to use FAKE_LABEL_NAME
> consistently, and adding a FAKE_LABEL_CHAR for the special character used to
> indicate a FAKE_LABEL_NAME. I fixed everything I found in the main gas
> sources, but did not change stuff in config, because it isn't broken if it
> doesn't redefine FAKE_LABEL_CHAR, and I can't easily test it.
>
> The second part is to stop accepting FAKE_LABEL_CHAR unless we are in a string.
> I noticed that I could put a \001 char in a source file, and gas would accept
> it, which is wrong. I added a testcase for that which fails without the patch
> and works with the patch. There is a \001 character in the testcase, hopefully
> that gets through email OK. This only affects the stab support.
>
> The third part is that I modified S_IS_LOCAL to check FAKE_LABEL_CHAR. This
> was working correctly only if FAKE_LABEL_NAME used the same special character
> as DOLLAR_LABEL_CHAR. This is something the riscv port got wrong, because it
> was an implicit assumption in the code. Now it is explicitly checking
> FAKE_LABEL_CHAR if different from DOLLAR_LABEL_CHAR.
>
> Tested with binutils/gas/ld builds and checks for riscv32-elf, riscv64-elf,
> and x86_64-linux. One less ld failure for the two riscv targets with the
> patch, and no regressions for x86-64.
>
> Comments?
Looks good to me, though I'm only really qualified to comment on the RISC-V
stuff.
>
> gas/
> * as.c: Include write.h.
> (common_emul_init): Use FAKE_LABEL_NAME.
> * ecoff.c (add_file, ecoff_directive_end, ecoff_directive_loc):
> Likewise.
> (ecoff_build_symbols): Use FAKE_LABEL_CHAR.
> * expr.c (get_symbol_name): Use FAKE_LABEL_CHAR. Accept only if
> input_from_string is TRUE.
> * read.c (input_from_string): New.
> (read_symbol_name): Use FAKE_LABEL_CHAR. Accept only if
> input_from_string is TRUE.
> (temp_ilp): Set input_from_string to TRUE.
> (restore_ilp): Set input_from_string to FALSE.
> * read.h (input_from_string): Declare.
> * symbols.c: Include write.h
> (S_IS_LOCAL): Check for FAKE_LABEL_CHAR.
> (symbol_relc_make_sym): Fix comment refering to default fake label
> string.
> * write.h (FAKE_LABEL_CHAR): New.
> * config/tc-riscv.h (FAKE_LABEL_CHAR): Define.
> * testsuite/gas/all/err-fakelabel.s: New.
> ---
> gas/as.c | 5 +++--
> gas/config/tc-riscv.h | 3 +++
> gas/ecoff.c | 10 +++++-----
> gas/expr.c | 7 ++++---
> gas/read.c | 9 ++++++---
> gas/read.h | 1 +
> gas/symbols.c | 6 +++++-
> gas/testsuite/gas/all/err-fakelabel.s | 3 +++
> gas/write.h | 6 ++++++
> 9 files changed, 36 insertions(+), 14 deletions(-)
> create mode 100644 gas/testsuite/gas/all/err-fakelabel.s
>
> diff --git a/gas/as.c b/gas/as.c
> index 31e62805af..fbf3257fbb 100644
> --- a/gas/as.c
> +++ b/gas/as.c
> @@ -39,6 +39,7 @@
> #include "dwarf2dbg.h"
> #include "dw2gencfi.h"
> #include "bfdver.h"
> +#include "write.h"
>
> #ifdef HAVE_ITBL_CPU
> #include "itbl-ops.h"
> @@ -199,10 +200,10 @@ common_emul_init (void)
> if (this_emulation->fake_label_name == 0)
> {
> if (this_emulation->leading_underscore)
> - this_emulation->fake_label_name = "L0\001";
> + this_emulation->fake_label_name = FAKE_LABEL_NAME;
> else
> /* What other parameters should we test? */
> - this_emulation->fake_label_name = ".L0\001";
> + this_emulation->fake_label_name = "." FAKE_LABEL_NAME;
> }
> }
> #endif
> diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
> index d79d30564c..04ef467b14 100644
> --- a/gas/config/tc-riscv.h
> +++ b/gas/config/tc-riscv.h
> @@ -39,6 +39,9 @@ struct expressionS;
> sure FAKE_LABEL_NAME is printable. It still must be distinct from any
> real label name. So, append a space, which other labels can't contain. */
> #define FAKE_LABEL_NAME ".L0 "
> +/* Changing the special character in FAKE_LABEL_NAME requires changing
> + FAKE_LABEL_CHAR too. */
> +#define FAKE_LABEL_CHAR ' '
>
> #define md_relax_frag(segment, fragp, stretch) \
> riscv_relax_frag (segment, fragp, stretch)
> diff --git a/gas/ecoff.c b/gas/ecoff.c
> index 325c3de362..2ec4076407 100644
> --- a/gas/ecoff.c
> +++ b/gas/ecoff.c
> @@ -2227,7 +2227,7 @@ add_file (const char *file_name, int indx ATTRIBUTE_UNUSED, int fake)
> if (stabs_seen)
> {
> (void) add_ecoff_symbol (file_name, st_Nil, sc_Nil,
> - symbol_new ("L0\001", now_seg,
> + symbol_new (FAKE_LABEL_NAME, now_seg,
> (valueT) frag_now_fix (),
> frag_now),
> (bfd_vma) 0, 0, ECOFF_MARK_STAB (N_SOL));
> @@ -3020,7 +3020,7 @@ ecoff_directive_end (int ignore ATTRIBUTE_UNUSED)
> as_warn (_(".end directive names unknown symbol"));
> else
> (void) add_ecoff_symbol ((const char *) NULL, st_End, sc_Text,
> - symbol_new ("L0\001", now_seg,
> + symbol_new (FAKE_LABEL_NAME, now_seg,
> (valueT) frag_now_fix (),
> frag_now),
> (bfd_vma) 0, (symint_t) 0, (symint_t) 0);
> @@ -3264,7 +3264,7 @@ ecoff_directive_loc (int ignore ATTRIBUTE_UNUSED)
> if (stabs_seen)
> {
> (void) add_ecoff_symbol ((char *) NULL, st_Label, sc_Text,
> - symbol_new ("L0\001", now_seg,
> + symbol_new (FAKE_LABEL_NAME, now_seg,
> (valueT) frag_now_fix (),
> frag_now),
> (bfd_vma) 0, 0, lineno);
> @@ -4104,10 +4104,10 @@ ecoff_build_symbols (const struct ecoff_debug_swap *backend,
> /* If an st_end symbol has an associated gas
> symbol, then it is a local label created for
> a .bend or .end directive. Stabs line
> - numbers will have \001 in the names. */
> + numbers will have FAKE_LABEL_CHAR in the names. */
> if (local
> && sym_ptr->ecoff_sym.asym.st != st_End
> - && strchr (sym_ptr->name, '\001') == 0)
> + && strchr (sym_ptr->name, FAKE_LABEL_CHAR) == 0)
> sym_ptr->ecoff_sym.asym.iss =
> add_string (&fil_ptr->strings,
> fil_ptr->str_hash,
> diff --git a/gas/expr.c b/gas/expr.c
> index 6fc707b8a5..fffb7f16ff 100644
> --- a/gas/expr.c
> +++ b/gas/expr.c
> @@ -2369,12 +2369,13 @@ get_symbol_name (char ** ilp_return)
> char c;
>
> * ilp_return = input_line_pointer;
> - /* We accept \001 in a name in case this is being called with a
> + /* We accept FAKE_LABEL_CHAR in a name in case this is being called with a
> constructed string. */
> - if (is_name_beginner (c = *input_line_pointer++) || c == '\001')
> + if (is_name_beginner (c = *input_line_pointer++)
> + || (input_from_string && c == FAKE_LABEL_CHAR))
> {
> while (is_part_of_name (c = *input_line_pointer++)
> - || c == '\001')
> + || (input_from_string && c == FAKE_LABEL_CHAR))
> ;
> if (is_name_ender (c))
> c = *input_line_pointer++;
> diff --git a/gas/read.c b/gas/read.c
> index afa1a168a6..6247b3ff58 100644
> --- a/gas/read.c
> +++ b/gas/read.c
> @@ -62,6 +62,7 @@
> #endif
>
> char *input_line_pointer; /*->next char of source file to parse. */
> +bfd_boolean input_from_string = FALSE;
>
> #if BITS_PER_CHAR != 8
> /* The following table is indexed by[(char)] and will break if
> @@ -1684,16 +1685,16 @@ read_symbol_name (void)
> if (mbstowcs (NULL, name, len) == (size_t) -1)
> as_warn (_("symbol name not recognised in the current locale"));
> }
> - else if (is_name_beginner (c) || c == '\001')
> + else if (is_name_beginner (c) || (input_from_string && c == FAKE_LABEL_CHAR))
> {
> ptrdiff_t len;
>
> name = input_line_pointer - 1;
>
> - /* We accept \001 in a name in case this is
> + /* We accept FAKE_LABEL_CHAR in a name in case this is
> being called with a constructed string. */
> while (is_part_of_name (c = *input_line_pointer++)
> - || c == '\001')
> + || (input_from_string && c == FAKE_LABEL_CHAR))
> ;
>
> len = (input_line_pointer - name) - 1;
> @@ -6385,6 +6386,7 @@ temp_ilp (char *buf)
>
> input_line_pointer = buf;
> buffer_limit = buf + strlen (buf);
> + input_from_string = TRUE;
> }
>
> /* Restore a saved input line pointer. */
> @@ -6396,6 +6398,7 @@ restore_ilp (void)
>
> input_line_pointer = saved_ilp;
> buffer_limit = saved_limit;
> + input_from_string = FALSE;
>
> saved_ilp = NULL;
> }
> diff --git a/gas/read.h b/gas/read.h
> index f16455d428..5bc7475484 100644
> --- a/gas/read.h
> +++ b/gas/read.h
> @@ -19,6 +19,7 @@
> 02110-1301, USA. */
>
> extern char *input_line_pointer; /* -> char we are parsing now. */
> +extern bfd_boolean input_from_string;
>
> /* Define to make whitespace be allowed in many syntactically
> unnecessary places. Normally undefined. For compatibility with
> diff --git a/gas/symbols.c b/gas/symbols.c
> index 19a1fa5728..19b3dbc667 100644
> --- a/gas/symbols.c
> +++ b/gas/symbols.c
> @@ -25,6 +25,7 @@
> #include "obstack.h" /* For "symbols.h" */
> #include "subsegs.h"
> #include "struc-symbol.h"
> +#include "write.h"
>
> /* This is non-zero if symbols are case sensitive, which is the
> default. */
> @@ -2161,6 +2162,9 @@ S_IS_LOCAL (symbolS *s)
> && ! S_IS_DEBUG (s)
> && (strchr (name, DOLLAR_LABEL_CHAR)
> || strchr (name, LOCAL_LABEL_CHAR)
> +#if FAKE_LABEL_CHAR != DOLLAR_LABEL_CHAR
> + || strchr (name, FAKE_LABEL_CHAR)
> +#endif
> || TC_LABEL_IS_LOCAL (name)
> || (! flag_keep_locals
> && (bfd_is_local_label (stdoutput, s->bsym)
> @@ -3087,7 +3091,7 @@ symbol_relc_make_sym (symbolS * sym)
> || S_GET_SEGMENT (sym) == absolute_section)
> return symbol_relc_make_expr (& sym->sy_value);
>
> - /* This may be a "fake symbol" L0\001, referring to ".".
> + /* This may be a "fake symbol", referring to ".".
> Write out a special null symbol to refer to this position. */
> if (! strcmp (S_GET_NAME (sym), FAKE_LABEL_NAME))
> return xstrdup (".");
> diff --git a/gas/testsuite/gas/all/err-fakelabel.s b/gas/testsuite/gas/all/err-fakelabel.s
> new file mode 100644
> index 0000000000..1e87f2ee2c
> --- /dev/null
> +++ b/gas/testsuite/gas/all/err-fakelabel.s
> @@ -0,0 +1,3 @@
> +;# Test that fake labels aren't accepted.
> +;# { dg-do assemble }
> +label: ;# { dg-error "Error: " }
> diff --git a/gas/write.h b/gas/write.h
> index a238320d4e..4157bc971c 100644
> --- a/gas/write.h
> +++ b/gas/write.h
> @@ -27,6 +27,12 @@
> #define FAKE_LABEL_NAME "L0\001"
> #endif
>
> +/* This is the special character used to indicate a fake symbol. Must be
> + present in FAKE_LABEL_NAME. */
> +#ifndef FAKE_LABEL_CHAR
> +#define FAKE_LABEL_CHAR '\001'
> +#endif
> +
> #include "bit_fix.h"
>
> /*
More information about the Binutils
mailing list