Add Rust support to source highlighting
Commit Message
Currently, no release of GNU Source Highlight supports Rust. However,
I've checked in a patch to do so there, and I plan to make a new
release sometime this summer.
This patch prepares gdb for that by adding support for Rust to the
source highlighting code.
Because Source Highlight will throw an exception if the language is
unrecognized, this also changes gdb to ignore exceptions here. This
will cause gdb to fall back to un-highlighted source text.
Tested with the current and development versions of Source Highlight.
gdb/ChangeLog
2019-06-24 Tom Tromey <tom@tromey.com>
* source-cache.c (get_language_name): Handle rust.
(source_cache::get_source_lines): Ignore highlighting exceptions.
---
gdb/ChangeLog | 5 +++++
gdb/source-cache.c | 33 ++++++++++++++++++++++-----------
2 files changed, 27 insertions(+), 11 deletions(-)
Comments
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> Currently, no release of GNU Source Highlight supports Rust. However,
Tom> I've checked in a patch to do so there, and I plan to make a new
Tom> release sometime this summer.
Tom> This patch prepares gdb for that by adding support for Rust to the
Tom> source highlighting code.
Tom> Because Source Highlight will throw an exception if the language is
Tom> unrecognized, this also changes gdb to ignore exceptions here. This
Tom> will cause gdb to fall back to un-highlighted source text.
Tom> Tested with the current and development versions of Source Highlight.
This patch works ok on Fedora 28, but on Fedora 29 it causes gdb to
crash.
I don't think there's anything wrong with the patch, so I guess this is
a Fedora bug somehow. I will see if I can write a test case and file
it.
Meanwhile I think this patch should not go in :-(
Tom
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> I don't think there's anything wrong with the patch, so I guess this is
Tom> a Fedora bug somehow. I will see if I can write a test case and file
Tom> it.
I debugged this a bit today, and the problem comes from the use of
"-static-libstdc++ -static-libgcc" when linking. Using just one of
these makes it work fine.
Are these necessary for some reason?
Tom
On 2019-07-01 11:40 a.m., Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>
> Tom> I don't think there's anything wrong with the patch, so I guess this is
> Tom> a Fedora bug somehow. I will see if I can write a test case and file
> Tom> it.
>
> I debugged this a bit today, and the problem comes from the use of
> "-static-libstdc++ -static-libgcc" when linking. Using just one of
> these makes it work fine.
>
> Are these necessary for some reason?
I never really knew the reason of using these in the first place.
Simon
On 7/2/19 1:29 AM, Simon Marchi wrote:
> On 2019-07-01 11:40 a.m., Tom Tromey wrote:
>>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>>
>> Tom> I don't think there's anything wrong with the patch, so I guess this is
>> Tom> a Fedora bug somehow. I will see if I can write a test case and file
>> Tom> it.
>>
>> I debugged this a bit today, and the problem comes from the use of
>> "-static-libstdc++ -static-libgcc" when linking. Using just one of
>> these makes it work fine.
>>
>> Are these necessary for some reason?
>
> I never really knew the reason of using these in the first place.
>
IIRC, they're there for gcc, so that a newly built gcc does not depend
on system libstdc++/libgcc or something along those lines.
I.e., it's mainly a consequence of a shared top level. I think that
the ideal would be if linking with static-* was controllable some
configure switch, and, if it was only enabled by default if building gcc.
Combined gdb + gcc builds must be considered as well, since some people
still do that.
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> IIRC, they're there for gcc, so that a newly built gcc does not depend
Pedro> on system libstdc++/libgcc or something along those lines.
Pedro> I.e., it's mainly a consequence of a shared top level. I think that
Pedro> the ideal would be if linking with static-* was controllable some
Pedro> configure switch, and, if it was only enabled by default if building gcc.
Pedro> Combined gdb + gcc builds must be considered as well, since some people
Pedro> still do that.
It seems like a bit of a pain to fix at top-level. These flags are put
into the default LDFLAGS for stage1, or when not bootstrapping.
How about just filtering them out in the gdb build? We could add a flag
to gdb's configure script to allow them to be added back in, on the off
chance that someone actually wants them.
Tom
On 7/13/19 5:06 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> IIRC, they're there for gcc, so that a newly built gcc does not depend
> Pedro> on system libstdc++/libgcc or something along those lines.
> Pedro> I.e., it's mainly a consequence of a shared top level. I think that
> Pedro> the ideal would be if linking with static-* was controllable some
> Pedro> configure switch, and, if it was only enabled by default if building gcc.
> Pedro> Combined gdb + gcc builds must be considered as well, since some people
> Pedro> still do that.
>
> It seems like a bit of a pain to fix at top-level. These flags are put
> into the default LDFLAGS for stage1, or when not bootstrapping.
>
Can you expand on why is it a pain? I was imagining that the top-level
script would take in consideration whether a gcc/ subdir exists, in
addition to checking some --{enable,disable}-static-runtime or some such,
where it adds the flags to LDFLAGS.
> How about just filtering them out in the gdb build? We could add a flag
> to gdb's configure script to allow them to be added back in, on the off
> chance that someone actually wants them.
I assume it is put in LDFLAGS for the whole tree in order to
use -static-libcc consistently for both gcc and the libraries it
depends on (like libiberty). (It'd be interesting to find the
rationale in the original mailing list post/patch that added it to
be sure.)
With what you're suggesting it sounds like we'd build libiberty/, libbfd/,
etc. with -static-libgcc and gdb/ without? That sounds like something
we shouldn't be doing either.
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>> It seems like a bit of a pain to fix at top-level. These flags are put
>> into the default LDFLAGS for stage1, or when not bootstrapping.
Pedro> Can you expand on why is it a pain? I was imagining that the top-level
Pedro> script would take in consideration whether a gcc/ subdir exists, in
Pedro> addition to checking some --{enable,disable}-static-runtime or some such,
Pedro> where it adds the flags to LDFLAGS.
The main problem is that the flags are passed down from the top-level
Makefile, so it would need extra top-level Makefile.* hacking.
Pedro> I assume it is put in LDFLAGS for the whole tree in order to
Pedro> use -static-libcc consistently for both gcc and the libraries it
Pedro> depends on (like libiberty). (It'd be interesting to find the
Pedro> rationale in the original mailing list post/patch that added it to
Pedro> be sure.)
Pedro> With what you're suggesting it sounds like we'd build libiberty/, libbfd/,
Pedro> etc. with -static-libgcc and gdb/ without? That sounds like something
Pedro> we shouldn't be doing either.
Are those even useful for libiberty or bfd? I thought those only
affected the link.
Or do people build a shared libiberty and/or bfd? That seems bad.
I tend to think -static-* is not ever correct for gdb, or at least is
incompatible with Source Highlight.
Tom
@@ -153,8 +153,7 @@ get_language_name (enum language lang)
break;
case language_rust:
- /* Not handled by Source Highlight. */
- break;
+ return "rust.lang";
case language_ada:
return "ada.lang";
@@ -223,18 +222,30 @@ source_cache::get_source_lines (struct symtab *s, int first_line,
highlighter->setStyleFile ("esc.style");
}
- std::ostringstream output;
- highlighter->highlight (input, output, lang_name, fullname);
+ try
+ {
+ std::ostringstream output;
+ highlighter->highlight (input, output, lang_name, fullname);
- source_text result = { fullname, output.str () };
- m_source_map.push_back (std::move (result));
+ source_text result = { fullname, output.str () };
+ m_source_map.push_back (std::move (result));
- if (m_source_map.size () > MAX_ENTRIES)
- m_source_map.erase (m_source_map.begin ());
+ if (m_source_map.size () > MAX_ENTRIES)
+ m_source_map.erase (m_source_map.begin ());
- *lines = extract_lines (m_source_map.back (), first_line,
- last_line);
- return true;
+ *lines = extract_lines (m_source_map.back (), first_line,
+ last_line);
+ return true;
+ }
+ catch (...)
+ {
+ /* Source Highlight will throw an exception if
+ highlighting fails. One possible reason it can
+ fail is if the language is unknown -- which
+ matters to gdb because Rust support wasn't added
+ until after 3.1.8. Ignore exceptions here an
+ fall back to un-highlighted text. */
+ }
}
}
}