Bug 33532 - bogus escape
Summary: bogus escape
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.3.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-09-23 06:27 UTC by Kai Henningsen
Modified: 2007-09-25 17:36 UTC (History)
3 users (show)

See Also:
Host:
Target: ia64
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-09-24 22:53:17


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kai Henningsen 2007-09-23 06:27:59 UTC
gcc/config/ia64/ia64.md:6267

{
  emit_library_call (gen_rtx_SYMBOL_REF (Pmode,
                                         \"__ia64_save_stack_nonlocal\"), <-----
                     0, VOIDmode, 2, XEXP (operands[0], 0), Pmode,
                     operands[1], Pmode);
  DONE;
})

This is in a brace block, NOT in a double quote string. These escapes are
bogus.

This is revision 127595.
Comment 1 Kai Henningsen 2007-09-23 06:37:08 UTC
Also line 6280:

{
  emit_library_call (gen_rtx_SYMBOL_REF (Pmode, \"__ia64_nonlocal_goto\"), <---
                     LCT_NORETURN, VOIDmode, 3,
                     operands[1], Pmode,
                     copy_to_reg (XEXP (operands[2], 0)), Pmode,
                     operands[3], Pmode);
  emit_barrier ();
  DONE;
})
Comment 2 Andrew Pinski 2007-09-23 06:42:24 UTC
This is not a bogus escape, just extraneous.
Comment 3 Wolfgang Bangerth 2007-09-23 16:56:55 UTC
(In reply to comment #2)
> This is not a bogus escape, just extraneous.

Uh, what exactly do you mean? This code is not valid:
-----------
char *p = \"abc\";
-----------
Neither should the one quoted be...

W.
Comment 4 Andrew Pinski 2007-09-23 16:59:58 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > This is not a bogus escape, just extraneous.
> 
> Uh, what exactly do you mean? This code is not valid:

this escape is in the machine description file which is not exactly C.
Comment 5 Jim Wilson 2007-09-24 22:53:16 UTC
Echoing what Andrew Pinski already said, this isn't C code, this is RTL code, the format of which is specified by the read-rtl.c file.  Specifically, see the read_brace_string function, which accepts backslash quoting exactly the same as the read_quoted_string function.  Unless you are proposing a change to the read_brace_string function, there is nothing wrong with this code.  Though I will admit that the backslashes are unnecessary here and could optionally be removed without harm.

If you are proposing changes to the read_brace_string function, it would have been nice to mention that.  If you want to find all such problems, changing this function is probably the easiest way to do that.  If you want to prevent such problems from being reintroduced later, then you would definitely have to change this function.  Maybe add an argument to the read_escape function so that we can handle calls from read_quoted_string differently than calls from read_brace_string?

I have no idea at the moment why this matters to you.  A little context might be helpful.
Comment 6 Kai Henningsen 2007-09-25 17:36:17 UTC
I don't know about Wolfgang.

As for me:

1. This is rather confusing.

2. I was writing a perl script to correlate source with tm.texi documentation. I certainly couldn't think of an algorithm that would parse all of these:

"......\"..."
\".....\"
\"...."
"...
..."

I believe I've even seen several of these in the same brace block. (Not necessarily in this target.)

I can certainly see no reason why these extra escapes SHOULD be permitted. All they seem to do is obfuscate the code.

Furthermore, this is most definitely undocumented (and I'd guess unintentional) behaviour. The docs seem to be pretty clear: the contents of a brace block is a piece of C, period.

Really, I think calling this an enhancment request is wrong: it is plainly a bug. Not necessarily a severe bug, and one can certainly argue why it's there and what would be the correct solution, but I am firmly convinced that the status quo is NOT correct.
Comment 7 bangerth@math.tamu.edu 2007-09-25 17:44:59 UTC
Subject: Re:  bogus escape


> I don't know about Wolfgang.

I was just confused, not realizing that we weren't in regular C code. Andrew's 
(as usual) brief comment didn't help the situation. So simply ignore my 
statement.

W.

-------------------------------------------------------------------------
Wolfgang Bangerth                email:            bangerth@math.tamu.edu
                                 www: http://www.math.tamu.edu/~bangerth/

Comment 8 Jim Wilson 2007-09-26 22:11:31 UTC
Subject: Re:  bogus escape

On Tue, 2007-09-25 at 17:36 +0000, kai-gcc-bugs at khms dot westfalen
dot de wrote:
> Furthermore, this is most definitely undocumented (and I'd guess unintentional)
> behaviour. The docs seem to be pretty clear: the contents of a brace block is a
> piece of C, period.

It is unintentional.  The original syntax was friendly to lisp parsers,
and we had only quoted-strings in define_insns, as lisp parsers can't
parse C code.  Later we added braced-strings in define_insns as a
simplification, which is no longer lisp friendly.  When this was done,
there was no change to the handling of escapes.  Internal to gcc, this
is still handled as if it is a string, it just doesn't start and end
with quote characters anymore.

The treatment of escapes is simple.  \" always simplifies to ",
regardless of whether you are inside or outside a string.  This is
because, originally, we were always inside a string here, and the
current code still behaves that way, even when using a braced-string.

> Really, I think calling this an enhancment request is wrong: it is plainly a
> bug. Not necessarily a severe bug, and one can certainly argue why it's there
> and what would be the correct solution, but I am firmly convinced that the
> status quo is NOT correct.

Except that the current behaviour causes no problems at any point in a
gcc build.  You can only see a problem because you have an external
program looking at md files.  This doesn't necessarily make it a gcc
bug.  It could be fixed just as easily in your program by changing how
you handle quoted characters.

I can create an artifical example that shows a gcc bug though.  If I add
this to the end of the ia64.md file:

(define_insn "artificial"
  [(unspec [(const_int 0)] 9999)]
  ""
{
  return ".section .artificial,\"r\",@progbits";
}
  [(set_attr "itanium_class" "ignore")
   (set_attr "predicable" "no")])

and then try to build it, I get an error building insn-output.o
../../gcc/gcc/config/ia64/ia64.md:6430: error: expected ‘;’ before ‘r’
This is because gcc translated this line to
  return ".section .artificial,"r",@progbits";
which is not valid C.  I can make this work by doubling the \
characters, but that is awkward.  We could declare this to be a real gcc
bug, file a PR for it, and then make this one depend on the new bug.
When this new bug gets fixed, this current bug will be exposed as a gcc
build failure, and will have to be fixed.