* [PATCH v2 0/2] Add Rust support to source highlighting
@ 2019-07-27 15:52 Tom Tromey
2019-07-27 15:52 ` [PATCH v2 2/2] " Tom Tromey
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Tom Tromey @ 2019-07-27 15:52 UTC (permalink / raw)
To: gdb-patches
This is v2 of the patch to add Rust support to source highlighting.
This version addresses the --static-libstdc++ problem that was found
with the previous patch, using the approach suggested by Pedro.
Let me know what you think. If this seems ok, my plan is to send the
top-level configury patch to gcc-patches for approval, and only then
check everything in. This plan avoids any problem if the gcc
maintainers request a different option name.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 2/2] Add Rust support to source highlighting 2019-07-27 15:52 [PATCH v2 0/2] Add Rust support to source highlighting Tom Tromey @ 2019-07-27 15:52 ` Tom Tromey 2019-09-04 17:22 ` Tom de Vries 2019-07-27 15:52 ` [PATCH v2 1/2] Add --with-static-standard-libraries to the top level Tom Tromey 2019-08-19 16:19 ` [PATCH v2 0/2] Add Rust support to source highlighting Tom Tromey 2 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2019-07-27 15:52 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey 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-07-27 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/configure b/gdb/configure index 12954d1f74a..cd92b70958c 100755 --- a/gdb/configure +++ b/gdb/configure @@ -11296,6 +11296,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 2a43d12df76..3dc4b7549b1 100644 --- a/gdb/configure.ac +++ b/gdb/configure.ac @@ -1217,6 +1217,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 f5bb641a22b..06a244ebbf0 100644 --- a/gdb/source-cache.c +++ b/gdb/source-cache.c @@ -154,8 +154,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"; @@ -224,18 +223,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 and + fall back to un-highlighted text. */ + } } } } -- 2.17.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] Add Rust support to source highlighting 2019-07-27 15:52 ` [PATCH v2 2/2] " Tom Tromey @ 2019-09-04 17:22 ` Tom de Vries 2019-09-10 15:56 ` Tom Tromey 0 siblings, 1 reply; 12+ messages in thread From: Tom de Vries @ 2019-09-04 17:22 UTC (permalink / raw) To: Tom Tromey, gdb-patches; +Cc: Pedro Alves On 27-07-19 17:51, Tom Tromey wrote: > 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. Hi, I recently updated my regular build setup to include an installed libsource-highlight.so by installing package libsource-highlight-devel in openSUSE Leap 15.1. Subsequently I ran into this error in the gdb 8.3 branch: ... $ ./install/bin/gdb -q a.out -ex start Reading symbols from a.out... Temporary breakpoint 1 at 0x40053b: file hello.c, line 9. Starting program: /data/gdb_versions/devel/a.out Temporary breakpoint 1, main () at hello.c:9 terminate called after throwing an instance of 'srchilite::ParserException' what(): error during the parsing of a definition file Aborted (core dumped) ... This expection happens when the library attempts to access /usr/share/source-highlight/esc.outlang, which is not there, because it's contained in another package (source-highlight). Installing that package fixes the error. Nevertheless, the error did not occur on master, and I've bisect that behaviour to this patch. So my question is: does it make sense to backport (part of) this patch to 8.3? Thanks, - Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] Add Rust support to source highlighting 2019-09-04 17:22 ` Tom de Vries @ 2019-09-10 15:56 ` Tom Tromey 2019-09-12 19:20 ` Tom de Vries 0 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2019-09-10 15:56 UTC (permalink / raw) To: Tom de Vries; +Cc: Tom Tromey, gdb-patches, Pedro Alves >>>>> "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. Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] Add Rust support to source highlighting 2019-09-10 15:56 ` Tom Tromey @ 2019-09-12 19:20 ` Tom de Vries 2019-09-17 18:24 ` Tom Tromey 0 siblings, 1 reply; 12+ messages in thread From: Tom de Vries @ 2019-09-12 19:20 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, Pedro Alves [-- Attachment #1: Type: text/plain, Size: 815 bytes --] 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 [-- Attachment #2: 0001-Add-with-static-standard-libraries-to-the-top-level.patch --] [-- Type: text/x-patch, Size: 4736 bytes --] 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) [-- Attachment #3: 0002-Add-Rust-support-to-source-highlighting.patch --] [-- Type: text/x-patch, Size: 5090 bytes --] 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. */ + } } } } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] Add Rust support to source highlighting 2019-09-12 19:20 ` Tom de Vries @ 2019-09-17 18:24 ` Tom Tromey 2019-09-18 22:16 ` Tom de Vries 0 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2019-09-17 18:24 UTC (permalink / raw) To: Tom de Vries; +Cc: Tom Tromey, gdb-patches, Pedro Alves >>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: Tom> Tested on openSUSE Leap 15.1, both with and without source-highlight Tom> package installed. Tom> OK for 8.3 branch? This looks fine to me, but I had two comments. First, Joel pointed out earlier that backports require a tracking PR, so be sure to file one and mention it in the commit. Second, up-thread you said: Tom> This expection happens when the library attempts to access Tom> /usr/share/source-highlight/esc.outlang, which is not there, because Tom> it's contained in another package (source-highlight). ... but the patch does: Tom> srchilite::SourceHighlight highlighter ("esc.outlang"); Tom> highlighter.setStyleFile("esc.style"); Tom> - std::ostringstream output; Tom> - highlighter.highlight (input, output, lang_name, fullname); Tom> + try Tom> + { Tom> + std::ostringstream output; Tom> + highlighter.highlight (input, output, lang_name, fullname); I am wondering if the "try" should encompass the construction of "highlighter". It's possible that it works fine as-is, since maybe the argument isn't used until highlighting is attempted -- but I figured I would ask just to be sure. thanks, Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] Add Rust support to source highlighting 2019-09-17 18:24 ` Tom Tromey @ 2019-09-18 22:16 ` Tom de Vries 2019-09-19 12:54 ` Tom Tromey 0 siblings, 1 reply; 12+ messages in thread From: Tom de Vries @ 2019-09-18 22:16 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, Pedro Alves [-- Attachment #1: Type: text/plain, Size: 1825 bytes --] On 17-09-19 20:23, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: > > Tom> Tested on openSUSE Leap 15.1, both with and without source-highlight > Tom> package installed. > > Tom> OK for 8.3 branch? > > This looks fine to me, but I had two comments. > > First, Joel pointed out earlier that backports require a tracking PR, so > be sure to file one and mention it in the commit. > Ack. Filed https://sourceware.org/bugzilla/show_bug.cgi?id=25009 "terminate called after throwing an instance of 'srchilite::ParserException'". > Second, up-thread you said: > > Tom> This expection happens when the library attempts to access > Tom> /usr/share/source-highlight/esc.outlang, which is not there, because > Tom> it's contained in another package (source-highlight). > > ... but the patch does: > > Tom> srchilite::SourceHighlight highlighter ("esc.outlang"); > Tom> highlighter.setStyleFile("esc.style"); > > Tom> - std::ostringstream output; > Tom> - highlighter.highlight (input, output, lang_name, fullname); > Tom> + try > Tom> + { > Tom> + std::ostringstream output; > Tom> + highlighter.highlight (input, output, lang_name, fullname); > > I am wondering if the "try" should encompass the construction of > "highlighter". It's possible that it works fine as-is, since maybe the > argument isn't used until highlighting is attempted -- but I figured I > would ask just to be sure. > Well, it does work, but your question makes me realize that that's an implementation artefact of the library, which could change for newer version (or perhaps there are older versions where that differs). So it should be safer to wrap highlighter construction as well. Updated the patch accordingly, and also added a PR number to both patches. Thanks, - Tom [-- Attachment #2: 0001-Add-with-static-standard-libraries-to-the-top-level.patch --] [-- Type: text/x-patch, Size: 4766 bytes --] 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> PR gdb/25009 * configure: Rebuild. * configure.ac: Add --with-static-standard-libraries. --- ChangeLog | 6 ++++++ configure | 24 +++++++++++++++++++++++- configure.ac | 16 +++++++++++++++- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index cd631a15b6..2441f4b014 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2019-08-19 Tom Tromey <tom@tromey.com> + + PR gdb/25009 + * 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) [-- Attachment #3: 0002-Add-Rust-support-to-source-highlighting.patch --] [-- Type: text/x-patch, Size: 5302 bytes --] 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> PR gdb/25009 * 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 | 9 +++++++++ gdb/configure | 6 ++++++ gdb/configure.ac | 8 ++++++++ gdb/source-cache.c | 37 ++++++++++++++++++++++++------------- 4 files changed, 47 insertions(+), 13 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 969515a65e..765e2c95e1 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2019-08-19 Tom Tromey <tom@tromey.com> + + PR gdb/25009 + * 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..d443c5e7fb 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"; @@ -213,21 +212,33 @@ source_cache::get_source_lines (struct symtab *s, int first_line, use-after-free. */ fullname = symtab_to_fullname (s); } - srchilite::SourceHighlight highlighter ("esc.outlang"); - highlighter.setStyleFile("esc.style"); + try + { + srchilite::SourceHighlight highlighter ("esc.outlang"); + highlighter.setStyleFile("esc.style"); - std::ostringstream output; - highlighter.highlight (input, output, lang_name, fullname); + 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. */ + } } } } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] Add Rust support to source highlighting 2019-09-18 22:16 ` Tom de Vries @ 2019-09-19 12:54 ` Tom Tromey 2019-09-19 16:18 ` [PATCH][gdb] Catch exception when constructing the highlighter Tom de Vries 0 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2019-09-19 12:54 UTC (permalink / raw) To: Tom de Vries; +Cc: Tom Tromey, gdb-patches, Pedro Alves >>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: Tom> Well, it does work, but your question makes me realize that that's an Tom> implementation artefact of the library, which could change for newer Tom> version (or perhaps there are older versions where that differs). So it Tom> should be safer to wrap highlighter construction as well. Tom> Updated the patch accordingly, and also added a PR number to both patches. Looks good to me -- but the bad news is you'll probably now want a similar patch on master. thanks, Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][gdb] Catch exception when constructing the highlighter 2019-09-19 12:54 ` Tom Tromey @ 2019-09-19 16:18 ` Tom de Vries 0 siblings, 0 replies; 12+ messages in thread From: Tom de Vries @ 2019-09-19 16:18 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, Pedro Alves [-- Attachment #1: Type: text/plain, Size: 799 bytes --] [ was: Re: [PATCH v2 2/2] Add Rust support to source highlighting ] On 19-09-19 14:54, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: > > Tom> Well, it does work, but your question makes me realize that that's an > Tom> implementation artefact of the library, which could change for newer > Tom> version (or perhaps there are older versions where that differs). So it > Tom> should be safer to wrap highlighter construction as well. > > Tom> Updated the patch accordingly, and also added a PR number to both patches. > > Looks good to me -- but the bad news is you'll probably now want a > similar patch on master. Ok, committed to 8.3 branch. Here's the proposed commit for master. Ok for trunk if testing on x86_64-linux finishes without problems? Thanks, - Tom [-- Attachment #2: 0001-gdb-Catch-exception-when-constructing-the-highlighter.patch --] [-- Type: text/x-patch, Size: 2015 bytes --] [gdb] Catch exception when constructing the highlighter Currently in source_cache::ensure we catch the exception that triggers when highlighter->highlight is called: ... try { std::istringstream input (contents); std::ostringstream output; highlighter->highlight (input, output, lang_name, fullname); ... and the file used earlier in the construction of the highlighter: ... highlighter = new srchilite::SourceHighlight ("esc.outlang"); ... is missing. The fact that this exception triggers when highlighter->highlight is called is an implementation artefact of libsource-highlight.so though, and this could be different for older or newer versions. Make things more robust by also catching exceptions thrown during construction of the highlighter. This makes the handling on master equivalent with what has been committed for 8.3.1. Tested on x86_64-linux. gdb/ChangeLog: 2019-09-19 Tom de Vries <tdevries@suse.de> PR gdb/25009 * source-cache.c (source_cache::ensure): Catch exception thrown during construction of the highlighter. --- gdb/source-cache.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gdb/source-cache.c b/gdb/source-cache.c index 7a52ce9458..1fe6da8132 100644 --- a/gdb/source-cache.c +++ b/gdb/source-cache.c @@ -190,14 +190,14 @@ source_cache::ensure (struct symtab *s) conditional compilation in source-cache.h. */ static srchilite::SourceHighlight *highlighter; - if (highlighter == nullptr) - { - highlighter = new srchilite::SourceHighlight ("esc.outlang"); - highlighter->setStyleFile ("esc.style"); - } - try { + if (highlighter == nullptr) + { + highlighter = new srchilite::SourceHighlight ("esc.outlang"); + highlighter->setStyleFile ("esc.style"); + } + std::istringstream input (contents); std::ostringstream output; highlighter->highlight (input, output, lang_name, fullname); ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] Add --with-static-standard-libraries to the top level 2019-07-27 15:52 [PATCH v2 0/2] Add Rust support to source highlighting Tom Tromey 2019-07-27 15:52 ` [PATCH v2 2/2] " Tom Tromey @ 2019-07-27 15:52 ` Tom Tromey 2019-08-19 16:19 ` [PATCH v2 0/2] Add Rust support to source highlighting Tom Tromey 2 siblings, 0 replies; 12+ messages in thread From: Tom Tromey @ 2019-07-27 15:52 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey 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-07-27 Tom Tromey <tom@tromey.com> * configure: Rebuild. * configure.ac: Add --with-static-standard-libraries. --- ChangeLog | 10 ++++++++++ configure | 24 +++++++++++++++++++++++- configure.ac | 16 +++++++++++++++- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 4d111486926..6a9719f6091 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 854f71a34e5..7433badc217 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) -- 2.17.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/2] Add Rust support to source highlighting 2019-07-27 15:52 [PATCH v2 0/2] Add Rust support to source highlighting Tom Tromey 2019-07-27 15:52 ` [PATCH v2 2/2] " Tom Tromey 2019-07-27 15:52 ` [PATCH v2 1/2] Add --with-static-standard-libraries to the top level Tom Tromey @ 2019-08-19 16:19 ` Tom Tromey 2019-08-19 18:29 ` Pedro Alves 2 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2019-08-19 16:19 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes: Tom> This is v2 of the patch to add Rust support to source highlighting. Tom> This version addresses the --static-libstdc++ problem that was found Tom> with the previous patch, using the approach suggested by Pedro. Tom> Let me know what you think. If this seems ok, my plan is to send the Tom> top-level configury patch to gcc-patches for approval, and only then Tom> check everything in. This plan avoids any problem if the gcc Tom> maintainers request a different option name. I've checked in the gcc patch, and I've updated the gdb patch to account for other recent changes to source-cache.c. I am going to check this in now. Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/2] Add Rust support to source highlighting 2019-08-19 16:19 ` [PATCH v2 0/2] Add Rust support to source highlighting Tom Tromey @ 2019-08-19 18:29 ` Pedro Alves 0 siblings, 0 replies; 12+ messages in thread From: Pedro Alves @ 2019-08-19 18:29 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 8/19/19 5:19 PM, Tom Tromey wrote: >>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes: > > Tom> This is v2 of the patch to add Rust support to source highlighting. > Tom> This version addresses the --static-libstdc++ problem that was found > Tom> with the previous patch, using the approach suggested by Pedro. > > Tom> Let me know what you think. If this seems ok, my plan is to send the > Tom> top-level configury patch to gcc-patches for approval, and only then > Tom> check everything in. This plan avoids any problem if the gcc > Tom> maintainers request a different option name. > > I've checked in the gcc patch, and I've updated the gdb patch to account > for other recent changes to source-cache.c. I am going to check this in > now. Thanks for doing this. I didn't realize until now that this series included such a patch, but reading it now I'm quite happy with it. Thanks for tackling that, and working with gcc upstream. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-09-19 16:18 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-27 15:52 [PATCH v2 0/2] Add Rust support to source highlighting Tom Tromey 2019-07-27 15:52 ` [PATCH v2 2/2] " Tom Tromey 2019-09-04 17:22 ` Tom de Vries 2019-09-10 15:56 ` Tom Tromey 2019-09-12 19:20 ` Tom de Vries 2019-09-17 18:24 ` Tom Tromey 2019-09-18 22:16 ` Tom de Vries 2019-09-19 12:54 ` Tom Tromey 2019-09-19 16:18 ` [PATCH][gdb] Catch exception when constructing the highlighter Tom de Vries 2019-07-27 15:52 ` [PATCH v2 1/2] Add --with-static-standard-libraries to the top level Tom Tromey 2019-08-19 16:19 ` [PATCH v2 0/2] Add Rust support to source highlighting Tom Tromey 2019-08-19 18:29 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).