Re: [PATCH 0/6] v2 of libdiagnostics
Hi David - and thanks for posting an outline for libdiagnostics at https://gcc.gnu.org/wiki/libdiagnostics Currently this shows both libdiagnosts and libdiagnostics-sarif-dump integrated into GCC. Is this the plan or would those be available as a top-level project (the program as an example for the library), possibly with the library sources also pushed to GCC? Oh, and one question as I stumbled over that today: Would libdiagnostics now (or in the future) use libtextstyle for formatting (and another possible sink: HTML)? Simon Am 23.11.2023 um 18:36 schrieb Pedro Alves: Hi David, On 2023-11-21 22:20, David Malcolm wrote: Here's v2 of the "libdiagnostics" shared library idea; see: https://gcc.gnu.org/wiki/libdiagnostics As in v1, patch 1 (for GCC) shows libdiagnostic.h (the public header file), along with examples of simple self-contained programs that show various uses of the API. As in v1, patch 2 (for GCC) is the work-in-progress implementation. Patch 3 (for GCC) adds a new libdiagnostics++.h, a wrapper API providing some syntactic sugar when using the API from C++. I've been using this to "eat my own dogfood" and write a simple SARIF-dumping tool: https://github.com/davidmalcolm/libdiagnostics-sarif-dump Patch 4 (for GCC) is an internal change needed by patch 1. Patch 5 (for GCC) updates GCC's source printing code so that when there's no column information, we don't print annotation lines. This fixes the extra lines seen using it from gas discussed in: https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635575.html Patch 6 (for binutils) is an updated version of the experiment at using the API from gas. Thoughts? Do you have plans on making this a top level library instead? That would allow easily making it a non-optional dependency for binutils, as we could have the library in the binutils-gdb repo as well, for instance. From the Cauldron discussion I understood that the diagnostics stuff doesn't depend on much of GCC's data structures, and doesn't rely on the garbage collector. Is there something preventing that? (Other than "it's-a-matter-of-time/effort", of course.) Pedro Alves
Re: [PATCH 0/6] v2 of libdiagnostics
Thank you for your efforts. Having the wiki page to track this definitely is useful! I'll have a look at the "real patch" later, likely next week. But for patch 4+5 which look quite clean: can we get an early improvement and inclusion into GCC for those? They only adjust internals and should be well covered by the existing test suite, so we may be able to inspect the other changes from this patchset "alone". Kind Regards, Simon Am 21.11.2023 um 23:20 schrieb David Malcolm: Here's v2 of the "libdiagnostics" shared library idea; see: https://gcc.gnu.org/wiki/libdiagnostics As in v1, patch 1 (for GCC) shows libdiagnostic.h (the public header file), along with examples of simple self-contained programs that show various uses of the API. As in v1, patch 2 (for GCC) is the work-in-progress implementation. Patch 3 (for GCC) adds a new libdiagnostics++.h, a wrapper API providing some syntactic sugar when using the API from C++. I've been using this to "eat my own dogfood" and write a simple SARIF-dumping tool: https://github.com/davidmalcolm/libdiagnostics-sarif-dump Patch 4 (for GCC) is an internal change needed by patch 1. Patch 5 (for GCC) updates GCC's source printing code so that when there's no column information, we don't print annotation lines. This fixes the extra lines seen using it from gas discussed in: https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635575.html Patch 6 (for binutils) is an updated version of the experiment at using the API from gas. Thoughts? David Malcolm (5): libdiagnostics v2: header and examples libdiagnostics v2: work-in-progress implementation libdiagnostics v2: add C++ wrapper API diagnostics: add diagnostic_context::get_location_text diagnostics: don't print annotation lines when there's no column info gcc/Makefile.in | 131 +- gcc/configure |2 +- gcc/configure.ac |2 +- gcc/diagnostic-show-locus.cc | 26 +- gcc/diagnostic.cc | 35 +- gcc/diagnostic.h |2 + gcc/libdiagnostics++.h| 378 + gcc/libdiagnostics.cc | 1306 + gcc/libdiagnostics.h | 602 gcc/libdiagnostics.map| 63 + .../libdiagnostics.dg/libdiagnostics.exp | 544 +++ gcc/testsuite/libdiagnostics.dg/test-dump.c | 55 + .../libdiagnostics.dg/test-error-with-note.c | 57 + .../libdiagnostics.dg/test-error-with-note.cc | 47 + gcc/testsuite/libdiagnostics.dg/test-error.c | 49 + gcc/testsuite/libdiagnostics.dg/test-error.cc | 40 + .../libdiagnostics.dg/test-fix-it-hint.c | 49 + .../libdiagnostics.dg/test-fix-it-hint.cc | 44 + .../libdiagnostics.dg/test-helpers++.h| 28 + .../libdiagnostics.dg/test-helpers.h | 29 + .../libdiagnostics.dg/test-labelled-ranges.c | 52 + .../libdiagnostics.dg/test-labelled-ranges.cc | 43 + .../libdiagnostics.dg/test-logical-location.c | 60 + .../libdiagnostics.dg/test-metadata.c | 54 + .../libdiagnostics.dg/test-multiple-lines.c | 61 + .../libdiagnostics.dg/test-no-column.c| 41 + .../test-note-with-fix-it-hint.c | 52 + .../test-text-sink-options.c | 46 + .../libdiagnostics.dg/test-warning.c | 52 + .../test-write-sarif-to-file.c| 46 + .../test-write-text-to-file.c | 47 + 31 files changed, 4018 insertions(+), 25 deletions(-) create mode 100644 gcc/libdiagnostics++.h create mode 100644 gcc/libdiagnostics.cc create mode 100644 gcc/libdiagnostics.h create mode 100644 gcc/libdiagnostics.map create mode 100644 gcc/testsuite/libdiagnostics.dg/libdiagnostics.exp create mode 100644 gcc/testsuite/libdiagnostics.dg/test-dump.c create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error-with-note.c create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error-with-note.cc create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error.c create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error.cc create mode 100644 gcc/testsuite/libdiagnostics.dg/test-fix-it-hint.c create mode 100644 gcc/testsuite/libdiagnostics.dg/test-fix-it-hint.cc create mode 100644 gcc/testsuite/libdiagnostics.dg/test-helpers++.h create mode 100644 gcc/testsuite/libdiagnostics.dg/test-helpers.h create mode 100644 gcc/testsuite/libdiagnostics.dg/test-labelled-ranges.c create mode 100644 gcc/testsuite/libdiagnostics.dg/test-labelled-ranges.cc create mode 100644 gcc/testsuite/libdiagnostics.dg/test-logical-location.c create mode 100644 gcc/testsuite/libdiagnostics.dg/test-metadata.c create mode 100644 gcc/testsuite/libdiagnostics.dg/test-multiple-lines.c create mode 100644 gcc/testsuite/libdiagnostics.dg/test-no-column.c
Re: [PATCH 2/2] libdiagnostics: work-in-progress implementation
Am 07.11.2023 um 15:59 schrieb David Malcolm: On Tue, 2023-11-07 at 08:54 +0100, Simon Sobisch wrote: Thank you for our work and providing this patch. GCC related questions: Is it planned to change GCC diagnostics to use libdiagnostic itself? No. GCC uses C++ internally, and the internal diagnostic API is written in C++. libdiagnostic wraps up this C++ API in a C interface. GCC would continue using the C++ interface internally. Why not providing both a C and C++ API in libdiagnostics? GNU programs (and also others) are written in both. The benefit of using it withing GCC itself "eat your own dogfood" would be that more or less any need that GCC has is also found in the library... thinking again, this may also make it "too heavy" - not sure. Is it planned to "directly" add features or would the result for GCC be identical (apart from build changes)? So far it looks like it wouldn't be possible to "just build libdiagnostics", and much less to "just distrubute its source" for that purpose, is it? Correct: libdiagnostics is just an extra .cc file within the rest of GCC, and almost all the work is being done in other .cc files. Maybe call that "status quo - initial patch"? ;-) As building GCC does take a significant amount of resources and system-wide switching to a new GCC version is considered a serious task (distributions commonly stay with their major GCC version for quite some time), I'd search for an option to building a "self-contained" version that does not need the complete necessary toolset and may also be distributed separately. It's possible to reduce the resources by disabling bootstrapping, and only enabling a minimal set of languages. I'd see libdiagnostics as coming from the distribution build of GCC. I suppose distributions might want to have a simple build of GCC and ship just the .so/.h file from libdiagnostics from the build. Agreed. But I'm as a "user" would like to have that "easy" option, too. As a maintainer that plans to move to libdiagnostics it would be _very_ helpful to be able to use it as a sub-project, in case it isn't available. This definitely can come later, too; I _guess_ this would mean moving part of GCCs code in a sub-folder libdiagnostics and use it as subproject for configure/make, with then option to run "make dist" in that subfolder alone, too. It would involve a lot of refactoring :) Something to "consider along, do later", I guess. The main reason for that would be to allow applications move from their previous own diagnostic to libdiagnostics, if it isn't available on the system they can build and install it as subproject, too; and to be able to build libdiagnostics with a much reduced dependency list. I can try to come up with a minimal recipe for building gcc if all you want is libdiagnostics Thanks, that already helps a lot. Simon
Re: [PATCH 2/2] libdiagnostics: work-in-progress implementation
Thank you for our work and providing this patch. GCC related questions: Is it planned to change GCC diagnostics to use libdiagnostic itself? Is it planned to "directly" add features or would the result for GCC be identical (apart from build changes)? So far it looks like it wouldn't be possible to "just build libdiagnostics", and much less to "just distrubute its source" for that purpose, is it? As building GCC does take a significant amount of resources and system-wide switching to a new GCC version is considered a serious task (distributions commonly stay with their major GCC version for quite some time), I'd search for an option to building a "self-contained" version that does not need the complete necessary toolset and may also be distributed separately. This definitely can come later, too; I _guess_ this would mean moving part of GCCs code in a sub-folder libdiagnostics and use it as subproject for configure/make, with then option to run "make dist" in that subfolder alone, too. The main reason for that would be to allow applications move from their previous own diagnostic to libdiagnostics, if it isn't available on the system they can build and install it as subproject, too; and to be able to build libdiagnostics with a much reduced dependency list. Thank you for considering that, Simon Am 06.11.2023 um 23:29 schrieb David Malcolm: Here's a work-in-progress patch for GCC that adds the implementation of libdiagnostics. Various aspects of this need work; posting now for early feedback on overall direction. For example, the testsuite doesn't yet check the output from the test client programs (and I'm not quite sure of the best way to express that in DejaGnu). gcc/ChangeLog: * Makefile.in (lang_checks): Add check-libdiagnostics. (start.encap): Add libdiagnostics. (libdiagnostics_OBJS): New. ...plus a bunch of stuff hacked up from jit/Make-lang.in. * configure: Regenerate. * configure.ac (check_languages): Add check-libdiagnostics. * input.h: Add FIXME. * libdiagnostics.cc: New file. * libdiagnostics.map: New file. gcc/testsuite/ChangeLog: * libdiagnostics.dg/libdiagnostics.exp: New, based on jit.exp. --- gcc/Makefile.in | 134 +- gcc/configure |2 +- gcc/configure.ac |2 +- gcc/input.h |2 +- gcc/libdiagnostics.cc | 1124 + gcc/libdiagnostics.map| 57 + .../libdiagnostics.dg/libdiagnostics.exp | 544 7 files changed, 1860 insertions(+), 5 deletions(-) create mode 100644 gcc/libdiagnostics.cc create mode 100644 gcc/libdiagnostics.map create mode 100644 gcc/testsuite/libdiagnostics.dg/libdiagnostics.exp diff --git a/gcc/Makefile.in b/gcc/Makefile.in index ff77d3cdc64..8f93ae48024 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -611,7 +611,7 @@ host_xm_defines=@host_xm_defines@ xm_file_list=@xm_file_list@ xm_include_list=@xm_include_list@ xm_defines=@xm_defines@ -lang_checks= +lang_checks=check-libdiagnostics lang_checks_parallelized= lang_opt_files=@lang_opt_files@ $(srcdir)/c-family/c.opt $(srcdir)/common.opt $(srcdir)/params.opt $(srcdir)/analyzer/analyzer.opt lang_specs_files=@lang_specs_files@ @@ -2153,7 +2153,7 @@ all.cross: native gcc-cross$(exeext) cpp$(exeext) specs \ libgcc-support lang.all.cross doc selftest @GENINSRC@ srcextra # This is what must be made before installing GCC and converting libraries. start.encap: native xgcc$(exeext) cpp$(exeext) specs \ - libgcc-support lang.start.encap @GENINSRC@ srcextra + libgcc-support lang.start.encap libdiagnostics @GENINSRC@ srcextra # These can't be made until after GCC can run. rest.encap: lang.rest.encap # This is what is made with the host's compiler @@ -2242,6 +2242,136 @@ cpp$(exeext): $(GCC_OBJS) c-family/cppspec.o libcommon-target.a $(LIBDEPS) \ c-family/cppspec.o $(EXTRA_GCC_OBJS) libcommon-target.a \ $(EXTRA_GCC_LIBS) $(LIBS) + +libdiagnostics_OBJS = libdiagnostics.o \ + libcommon.a + +# FIXME: +# Define the names for selecting jit in LANGUAGES. +# Note that it would be nice to move the dependency on g++ +# into the jit rule, but that needs a little bit of work +# to do the right thing within all.cross. + +LIBDIAGNOSTICS_VERSION_NUM = 0 +LIBDIAGNOSTICS_MINOR_NUM = 0 +LIBDIAGNOSTICS_RELEASE_NUM = 1 + +ifneq (,$(findstring mingw,$(target))) +LIBDIAGNOSTICS_FILENAME = libdiagnostics-$(LIBDIAGNOSTICS_VERSION_NUM).dll +LIBDIAGNOSTICS_IMPORT_LIB = libdiagnostics.dll.a + +libdiagnostics: $(LIBDIAGNOSTICS_FILENAME) \ + $(FULL_DRIVER_NAME) + +else + +ifneq (,$(findstring darwin,$(host))) + +LIBDIAGNOSTICS_AGE = 1 +LIBDIAGNOSTICS_BASENAME = libdiagnostics + +LIBDIAGNOSTICS_SONAME = \ +
Re: [PATCH] binutils: experimental use of libdiagnostics in gas
Thank you very much for this proof-of-concept use!
Inspecting it raises the following questions to me, both for a possible
binutils implementation and for the library use in general:
* How should the application set the relevant context (often lines are
shown before/after)?
* Should it be possible to override msgid used to display the
warning/error type?
If this would be possible then the text sink in messages_init may be
adjusted to replace the label with _("Warning") and _("Error"), which
would leave the text output "as-is" (if the text sink is configured to
not output the source line); this would make it usable without
adjusting the testsuite and to adopt to a standard later.
Notes for the SARIF output:
* the region contains an error, according to the linked JSON spec
startColumn has a minimum of 1 (I guess you'd just leave it away if
the application did not set it)
* the application should have the option to pre-set the sourceLanguage
for the diagnostic_manager (maybe even make that a positional argument
that needs to be passed but can be NULL) and override it when
specifying a region
Thanks,
Simon
Am 06.11.2023 um 23:29 schrieb David Malcolm:
Here's a patch for gas in binutils that makes it use libdiagnostics
(with some nasty hardcoded paths to specific places on my hard drive
to make it easier to develop the API).
For now this hardcodes adding two sinks: a text sink on stderr, and
also a SARIF output to stderr (which happens after all regular output).
For example, without this patch:
gas testsuite/gas/all/warn-1.s
emits:
testsuite/gas/all/warn-1.s: Assembler messages:
testsuite/gas/all/warn-1.s:3: Warning: a warning message
testsuite/gas/all/warn-1.s:4: Error: .warning argument must be a string
testsuite/gas/all/warn-1.s:5: Warning: .warning directive invoked in source file
testsuite/gas/all/warn-1.s:6: Warning: .warning directive invoked in source file
testsuite/gas/all/warn-1.s:7: Warning:
whereas with this patch:
LD_LIBRARY_PATH=/home/david/coding-3/gcc-newgit-canvas-2023/build/gcc
./as-new testsuite/gas/all/warn-1.s
emits:
testsuite/gas/all/warn-1.s:3: warning: a warning message
3 | .warning "a warning message" ;# { dg-warning "Warning: a warning
message" }
|
testsuite/gas/all/warn-1.s:4: error: .warning argument must be a string
4 | .warning a warning message ;# { dg-error "Error: .warning argument
must be a string" }
|
testsuite/gas/all/warn-1.s:5: warning: .warning directive invoked in source file
5 | .warning ;# { dg-warning "Warning: .warning
directive invoked in source file" }
|
testsuite/gas/all/warn-1.s:6: warning: .warning directive invoked in source file
6 | .warning ".warning directive invoked in source file" ;# { dg-warning
"Warning: .warning directive invoked in source file" }
|
testsuite/gas/all/warn-1.s:7: warning:
7 | .warning "";# { dg-warning "Warning: " }
|
{"$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json;, "version": "2.1.0", "runs": [{"tool": {"driver": {"rules": []}}, "invocations": [{"executionSuccessful": true, "toolExecutionNotifications": []}], "originalUriBaseIds": {"PWD": {"uri": "file:///home/david/coding-3/binutils-gdb/gas/"}}, "artifacts": [{"location": {"uri": "testsuite/gas/all/warn-1.s", "uriBaseId": "PWD"}, "contents": {"text": ";# Test .warning directive.\n;# { dg-do assemble }\n .warning \"a warning message\"\t;# { dg-warning \"Warning: a warning message\" }\n .warning a warning message\t;# { dg-error \"Error: .warning argument must be a string\" }\n .warning\t\t\t;# { dg-warning \"Warning: .warning directive invoked in source file\" }\n .warning \".warning directive invoked in source file\"\t;# { dg-warning \"Warning: .warning directive invoked in source file\" }\n .warning \"\"\t\t\t;# { dg-warning \"Warning: \" }\n"}}], "results": [{"ruleId": "warning", "level": "warning", "message": {"text": "a warning
message"}, "locations": [{"physicalLocation": {"artifactLocation": {"uri": "testsuite/gas/all/warn-1.s", "uriBaseId": "PWD"}, "region": {"startLine": 3, "startColumn": 0, "endColumn": 1}, "contextRegion": {"startLine": 3, "snippet": {"text": " .warning \"a warning message\"\t;# { dg-warning \"Warning: a warning message\" }\n"], "relatedLocations": [{"physicalLocation": {"artifactLocation": {"uri": "testsuite/gas/all/warn-1.s", "uriBaseId": "PWD"}, "region": {"startLine": 4, "startColumn": 0, "endColumn": 1}, "contextRegion": {"startLine": 4, "snippet": {"text": " .warning a warning message\t;# { dg-error \"Error: .warning argument must be a string\" }\n"}}}, "message": {"text": ".warning argument
Re: libiberty: Would it be reasonable to add support for GnuCOBOL function name demangling?
Am 27.05.22 um 20:31 schrieb Eric Gallager: On Fri, May 27, 2022 at 3:17 AM Simon Sobisch via Gcc-patches wrote: [...] the first question is: is it reasonable to add support for GnuCOBOL? * How would the demangler know it is to be called? Just "best match" (GnuCOBOL modules always have some symbols in it which should be available if there is any debugging information in, if that helps)? * Giving the work of gcc-cobol which was discussed on this mailing list some months ago (not sure about its current state) there possibly will be a COBOL support be "integrated" - with possibly different name mangling. But still - GnuCOBOL is used "in the wild" (for production environments) since years (and will be for many years to come, both based on GCC and with other compilers) and the name mangling rules did not change. If the plan is to integrate GnuCOBOL into trunk, then I'd say adding demangling support for it to libiberty would not only be reasonable, but also a necessary prerequisite for merging the rest of it. Just to ensure that there aren't confusions: Nobody intends to integrate GnuCOBOL [0] into gcc; but it would be important for gcobol for being integrated into gcc to succeed. GnuCOBOL (formerly OpenCOBOL) is a project which translates COBOL to intermediate C (mostly consisting of calls to functions in the GnuCOBOL runtime library libcob), then executes the "native" / system C compiler. It is very mature and used a lot; we _suggest_ to use GCC but also work with other free and nonfree compilers on free and nonfree systems. gcobol [1][2] (I've also seen it referenced as gcc-cobol) is an actual gcc frontend, so translates into gcc intermediate format. As far as I know, the plans are to both provide a usable working COBOL compiler and reach a state for integration until 2023. It possibly will use a very small but important part of libcob (at least if available) to provide support of a COBOL-native way to read/write data. When it comes up to the integration phase it _could_ be considered to integrate only those parts as-is (so effectively forking libcob to glibcob), as both GCC and GnuCOBOL are FSF-Copyrighted - or add it as an optional dependency (a lot of COBOL users don't use that 'old' way of accessing data and moved to EXEC SQL preprocessors instead). But as GnuCOBOL maintainer my question here was about the GnuCOBOL name mangling. I've now learned that as there isn't an explicit prefix like Z_ the de-mangling will be an "upon request", and as far as current responses were it seems like an reasonable approach and "patches to add that are likely to be accepted" (otherwise I won't start, because obviously there is always something to do on the GnuCOBOL side, too). Simon [0]: http://www.gnu.org/software/gnucobol [1]: https://gcc.gnu.org/pipermail/gcc/2022-March/238408.html [2]: https://git.symas.net/cobolworx/gcc-cobol/-/tree/master+cobol/gcc/cobol
libiberty: Would it be reasonable to add support for GnuCOBOL function name demangling?
Hi fellow hackers, first of all: I'm not sure if this is the correct mailing list for this question, but I did not found a separate one and gnu.org/software/libiberty redirects to https://gcc.gnu.org/onlinedocs/libiberty.pdf - so I'm here. If there's a better place for this: please drop a note. I've never "worked" with libiberty directly but am sure I'm using it quite regularly with various tools including GDB and valgrind. Therefore I currently cannot send a patch for the function name demangling, but if this is a reasonable thing to add I'd like to work on this with someone. As noted: the first question is: is it reasonable to add support for GnuCOBOL? * How would the demangler know it is to be called? Just "best match" (GnuCOBOL modules always have some symbols in it which should be available if there is any debugging information in, if that helps)? * Giving the work of gcc-cobol which was discussed on this mailing list some months ago (not sure about its current state) there possibly will be a COBOL support be "integrated" - with possibly different name mangling. But still - GnuCOBOL is used "in the wild" (for production environments) since years (and will be for many years to come, both based on GCC and with other compilers) and the name mangling rules did not change. A second question would be: Is there anyone who would be willing to work on this with me? Where would "we" or I start? Thank you for taking the time to read and possibly answer, Simon Sobisch Maintainer GnuCOBOL OpenPGP_0x13E96B53C005604E.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
