Re: [PATCH v2 2/2] Add Rust support to source highlighting

[Tom de Vries <tdevries at suse dot de>]

On 10-09-19 17:56, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> I recently updated my regular build setup to include an installed
> Tom> libsource-highlight.so by installing package libsource-highlight-devel
> Tom> in openSUSE Leap 15.1.
> ...
> 
> Tom> So my question is: does it make sense to backport (part of) this patch
> Tom> to 8.3?
> 
> It would be fine by me.  The configury bits would also be needed.

So, commit c1a5d03a89 "Add --with-static-standard-libraries to the top
level" applied cleanly, but commit d806ea2d0e "Add Rust support to
source highlighting" didn't so I'd like a review of that one.

Attached both backported patches here.

Tested on openSUSE Leap 15.1, both with and without source-highlight
package installed.

OK for 8.3 branch?

Thanks,
- Tom

Add --with-static-standard-libraries to the top level

[ Backport of master commit c1a5d03a89. ]

gdb should normally not be linked with -static-libstdc++.  Currently
this has not caused problems, but it's incompatible with catching an
exception thrown from a shared library -- and a subsequent patch
changes gdb to do just this.

This patch adds a new --with-static-standard-libraries flag to the
top-level configure.  It defaults to "auto", which means enabled if
gcc is being built, and disabled otherwise.

ChangeLog
2019-08-19  Tom Tromey  <tom@tromey.com>

	* configure: Rebuild.
	* configure.ac: Add --with-static-standard-libraries.

---
 ChangeLog    |  5 +++++
 configure    | 24 +++++++++++++++++++++++-
 configure.ac | 16 +++++++++++++++-
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index cd631a15b6..fb30d03109 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2019-08-19  Tom Tromey  <tom@tromey.com>
+
+	* configure: Rebuild.
+	* configure.ac: Add --with-static-standard-libraries.
+
 2018-06-24  Nick Clifton  <nickc@redhat.com>
 
 	2.32 branch created.
diff --git a/configure b/configure
index 3747645961..0a500810e3 100755
--- a/configure
+++ b/configure
@@ -802,6 +802,7 @@ with_gmp
 with_gmp_include
 with_gmp_lib
 with_stage1_libs
+with_static_standard_libraries
 with_stage1_ldflags
 with_boot_libs
 with_boot_ldflags
@@ -1572,6 +1573,9 @@ Optional Packages:
   --with-gmp-include=PATH specify directory for installed GMP include files
   --with-gmp-lib=PATH     specify directory for the installed GMP library
   --with-stage1-libs=LIBS libraries for stage1
+  --with-static-standard-libraries
+                          use -static-libstdc++ and -static-libgcc
+                          (default=auto)
   --with-stage1-ldflags=FLAGS
                           linker flags for stage1
   --with-boot-libs=LIBS   libraries for stage2 and later
@@ -5824,6 +5828,23 @@ fi
 
 
 
+# Whether or not to use -static-libstdc++ and -static-libgcc.  The
+# default is yes if gcc is being built; no otherwise.  The reason for
+# this default is that gdb is sometimes linked against GNU Source
+# Highlight, which is a shared library that uses C++ exceptions.  In
+# this case, -static-libstdc++ will cause crashes.
+
+# Check whether --with-static-standard-libraries was given.
+if test "${with_static_standard_libraries+set}" = set; then :
+  withval=$with_static_standard_libraries;
+else
+  with_static_standard_libraries=auto
+fi
+
+if test "$with_static_standard_libraries" = auto; then
+  with_static_standard_libraries=$have_compiler
+fi
+
 # Linker flags to use for stage1 or when not bootstrapping.
 
 # Check whether --with-stage1-ldflags was given.
@@ -5838,7 +5859,8 @@ else
  # In stage 1, default to linking libstdc++ and libgcc statically with GCC
  # if supported.  But if the user explicitly specified the libraries to use,
  # trust that they are doing what they want.
- if test "$stage1_libs" = "" -a "$have_static_libs" = yes; then
+ if test "$with_static_standard_libraries" = yes -a "$stage1_libs" = "" \
+     -a "$have_static_libs" = yes; then
    stage1_ldflags="-static-libstdc++ -static-libgcc"
  fi
 fi
diff --git a/configure.ac b/configure.ac
index 46501c2882..6cbee2ae2c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1602,6 +1602,19 @@ AC_ARG_WITH(stage1-libs,
 [stage1_libs=])
 AC_SUBST(stage1_libs)
 
+# Whether or not to use -static-libstdc++ and -static-libgcc.  The
+# default is yes if gcc is being built; no otherwise.  The reason for
+# this default is that gdb is sometimes linked against GNU Source
+# Highlight, which is a shared library that uses C++ exceptions.  In
+# this case, -static-libstdc++ will cause crashes.
+AC_ARG_WITH(static-standard-libraries,
+[AS_HELP_STRING([--with-static-standard-libraries],
+                [use -static-libstdc++ and -static-libgcc (default=auto)])],
+[], [with_static_standard_libraries=auto])
+if test "$with_static_standard_libraries" = auto; then
+  with_static_standard_libraries=$have_compiler
+fi
+
 # Linker flags to use for stage1 or when not bootstrapping.
 AC_ARG_WITH(stage1-ldflags,
 [AS_HELP_STRING([--with-stage1-ldflags=FLAGS], [linker flags for stage1])],
@@ -1614,7 +1627,8 @@ AC_ARG_WITH(stage1-ldflags,
  # In stage 1, default to linking libstdc++ and libgcc statically with GCC
  # if supported.  But if the user explicitly specified the libraries to use,
  # trust that they are doing what they want.
- if test "$stage1_libs" = "" -a "$have_static_libs" = yes; then
+ if test "$with_static_standard_libraries" = yes -a "$stage1_libs" = "" \
+     -a "$have_static_libs" = yes; then
    stage1_ldflags="-static-libstdc++ -static-libgcc"
  fi])
 AC_SUBST(stage1_ldflags)

Add Rust support to source highlighting

[ Backport of master commit d806ea2d0e. ]

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.

This updates gdb's configure script to reject the combination of
Source Highlight and -static-libstdc++.  This is done because it's not
possible to use -static-libstdc++ and then catch exceptions from a
shared library.

Tested with the current and development versions of Source Highlight.

gdb/ChangeLog
2019-08-19  Tom Tromey  <tom@tromey.com>

	* configure: Rebuild.
	* configure.ac: Disallow the combination of -static-libstdc++ and
	source highlight.
	* source-cache.c (get_language_name): Handle rust.
	(source_cache::get_source_lines): Ignore highlighting exceptions.

---
 gdb/ChangeLog      |  8 ++++++++
 gdb/configure      |  6 ++++++
 gdb/configure.ac   |  8 ++++++++
 gdb/source-cache.c | 33 ++++++++++++++++++++++-----------
 4 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fd3a86cbcf..76fcda3ba7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-08-19  Tom Tromey  <tom@tromey.com>
+
+	* configure: Rebuild.
+	* configure.ac: Disallow the combination of -static-libstdc++ and
+	source highlight.
+	* source-cache.c (get_language_name): Handle rust.
+	(source_cache::get_source_lines): Ignore highlighting exceptions.
+
 2019-06-28  Sergio Durigan Junior  <sergiodj@redhat.com>
 
 	PR breakpoints/24541
diff --git a/gdb/configure b/gdb/configure
index 854837c50a..866564fbe6 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -11503,6 +11503,12 @@ $as_echo "no - pkg-config not found" >&6; }
       as_fn_error $? "pkg-config was not found in your system" "$LINENO" 5
     fi
   else
+    case "$LDFLAGS" in
+      *static-libstdc*)
+        as_fn_error $? "source highlight is incompatible with -static-libstdc++; either use --disable-source-highlight or --without-static-standard-libraries" "$LINENO" 5
+        ;;
+    esac
+
     if ${pkg_config_prog_path} --exists source-highlight; then
       SRCHIGH_CFLAGS=`${pkg_config_prog_path} --cflags source-highlight`
       SRCHIGH_LIBS=`${pkg_config_prog_path} --libs source-highlight`
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 1527585839..0805827adf 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1251,6 +1251,14 @@ if test "${enable_source_highlight}" != "no"; then
       AC_MSG_ERROR([pkg-config was not found in your system])
     fi
   else
+    case "$LDFLAGS" in
+      *static-libstdc*)
+        AC_MSG_ERROR([source highlight is incompatible with -static-libstdc++; dnl
+either use --disable-source-highlight or dnl
+--without-static-standard-libraries])
+        ;;
+    esac
+
     if ${pkg_config_prog_path} --exists source-highlight; then
       SRCHIGH_CFLAGS=`${pkg_config_prog_path} --cflags source-highlight`
       SRCHIGH_LIBS=`${pkg_config_prog_path} --libs source-highlight`
diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index 5eae02082d..77f92879fd 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -156,8 +156,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";
@@ -216,18 +215,30 @@ source_cache::get_source_lines (struct symtab *s, int first_line,
 	      srchilite::SourceHighlight highlighter ("esc.outlang");
 	      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 and fall back to
+		     un-highlighted text. */
+		}
 	    }
 	}
     }