binutils: v2: experimental use of libdiagnostics in gas
Checks
Commit Message
Changed in v2:
* updated for change from diagnostic_location_t to
const diagnostic_physical_location *
* fix #if USE_DIAGNOSTICS to retain context and listing code
Output from the example below with v2 is now:
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: " }
and there's also now a way to toggle quoting of the source code (perhaps
for use in the testsuite)
Blurb from v1:
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:
VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
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:
VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
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 must be a string"}}, {"physicalLocation": {"artifactLocation": {"uri": "testsuite/gas/all/warn-1.s", "uriBaseId": "PWD"}, "region": {"startLine": 5, "startColumn": 0, "endColumn": 1}, "contextRegion": {"startLine": 5, "snippet": {"text": " .warning\t\t\t;# { dg-warning \"Warning: .warning directive invoked in source file\" }\n"}}}, "message": {"text": ".warning directive invoked in source file"}}, {"physicalLocation": {"artifactLocation": {"uri": "testsuite/gas/all/warn-1.s", "uriBaseId": "PWD"}, "region": {"startLine": 6, "startColumn": 0, "endColumn": 1}, "contextRegion": {"startLine": 6, "snippet": {"text": " .warning \".warning directive invoked in source file\"\t;# { dg-warning \"Warning: .warning directive invoked in source file\" }\n"}}}, "message": {"text": ".warning directive invoked in source file"}}, {"physicalLocation": {"artifactLocation": {"uri": "testsuite/gas/all/warn-1.s", "uriBaseId": "PWD"}, "region": {"startLine": 7, "startColumn": 0, "endColumn": 1}, "contextRegion": {"startLine": 7, "snippet": {"text": " .warning \"\"\t\t\t;# { dg-warning \"Warning: \" }\n"}}}, "message": {"text": ""}}]}]}]}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
For reference, running that SARIF through "python -m json.tool" to
prettyify it gives:
VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
{
"$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 must be a string"
}
},
{
"physicalLocation": {
"artifactLocation": {
"uri": "testsuite/gas/all/warn-1.s",
"uriBaseId": "PWD"
},
"region": {
"startLine": 5,
"startColumn": 0,
"endColumn": 1
},
"contextRegion": {
"startLine": 5,
"snippet": {
"text": " .warning\t\t\t;# { dg-warning \"Warning: .warning directive invoked in source file\" }\n"
}
}
},
"message": {
"text": ".warning directive invoked in source file"
}
},
{
"physicalLocation": {
"artifactLocation": {
"uri": "testsuite/gas/all/warn-1.s",
"uriBaseId": "PWD"
},
"region": {
"startLine": 6,
"startColumn": 0,
"endColumn": 1
},
"contextRegion": {
"startLine": 6,
"snippet": {
"text": " .warning \".warning directive invoked in source file\"\t;# { dg-warning \"Warning: .warning directive invoked in source file\" }\n"
}
}
},
"message": {
"text": ".warning directive invoked in source file"
}
},
{
"physicalLocation": {
"artifactLocation": {
"uri": "testsuite/gas/all/warn-1.s",
"uriBaseId": "PWD"
},
"region": {
"startLine": 7,
"startColumn": 0,
"endColumn": 1
},
"contextRegion": {
"startLine": 7,
"snippet": {
"text": " .warning \"\"\t\t\t;# { dg-warning \"Warning: \" }\n"
}
}
},
"message": {
"text": ""
}
}
]
}
]
}
]
}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
gas/ChangeLog:
* Makefile.am (GASLIBS): Add nasty hack with hardcoded path
on my hard drive.
* Makefile.in (GASLIBS): Likewise.
* as.c (gas_init): Call messages_init.
(main): Call messages_end.
* as.h (messages_init): New decl.
(messages_end): New decl.
* messages.c (USE_LIBDIAGNOSTICS): New define.
Add #include with nasty hardcoded path.
(diag_mgr): New.
(messages_init): New.
(messages_end): New.
(as_warn_internal): Add support for libdiagnostics.
(as_bad_internal): Likewise.
---
gas/Makefile.am | 3 ++-
gas/Makefile.in | 4 +++-
gas/as.c | 3 +++
gas/as.h | 3 +++
gas/messages.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 71 insertions(+), 2 deletions(-)
Comments
On 21.11.2023 23:20, David Malcolm wrote:
> @@ -101,6 +109,29 @@ had_warnings (void)
> return warning_count;
> }
>
> +#if USE_LIBDIAGNOSTICS
> +static diagnostic_manager *diag_mgr;
> +#endif
> +
> +void messages_init (void)
> +{
> +#if USE_LIBDIAGNOSTICS
> + diag_mgr = diagnostic_manager_new ();
> + diagnostic_manager_add_text_sink (diag_mgr, stderr,
> + DIAGNOSTIC_COLORIZE_IF_TTY);
Coloring will want to be possible to turn off (or maybe be off by default,
and be possible to turn on).
As to the #if-s: I think they all would better be #ifdef.
> @@ -182,6 +213,20 @@ as_warn_internal (const char *file, unsigned int line, char *buffer)
> context = true;
> }
>
> +#if USE_LIBDIAGNOSTICS
> + const diagnostic_file *file_obj
> + = diagnostic_manager_new_file (diag_mgr, file, NULL);
> +
> + const diagnostic_physical_location *loc
> + = diagnostic_manager_new_location_from_file_and_line (diag_mgr,
> + file_obj,
> + line);
> +
> + diagnostic *d = diagnostic_begin (diag_mgr,
> + DIAGNOSTIC_LEVEL_WARNING);
> + diagnostic_set_location (d, loc);
> + diagnostic_finish (d, "%s", buffer);
> +#else
This looks identical to ...
> @@ -256,6 +302,19 @@ as_bad_internal (const char *file, unsigned int line, char *buffer)
> context = true;
> }
>
> +#if USE_LIBDIAGNOSTICS
> + const diagnostic_file *file_obj
> + = diagnostic_manager_new_file (diag_mgr, file, NULL);
> + const diagnostic_physical_location *loc
> + = diagnostic_manager_new_location_from_file_and_line (diag_mgr,
> + file_obj,
> + line);
> +
> + diagnostic *d = diagnostic_begin (diag_mgr,
> + DIAGNOSTIC_LEVEL_ERROR);
> + diagnostic_set_location (d, loc);
> + diagnostic_finish (d, "%s", buffer);
> +#else
... this, except for the level used. Put into a helper function? Which
would the further want using from as_info_where()?
Jan
@@ -408,7 +408,8 @@ AM_CPPFLAGS = -I. -I$(srcdir) -I../bfd -I$(srcdir)/config \
# How to link with both our special library facilities
# and the system's installed libraries.
-GASLIBS = @OPCODES_LIB@ ../bfd/libbfd.la ../libiberty/libiberty.a
+# FIXME:
+GASLIBS = @OPCODES_LIB@ ../bfd/libbfd.la ../libiberty/libiberty.a /home/david/coding-3/gcc-newgit-canvas-2023/build/gcc/libdiagnostics.so
# Files to be copied away after each stage in building.
STAGESTUFF = *.@OBJEXT@ $(noinst_PROGRAMS)
@@ -885,7 +885,9 @@ AM_CPPFLAGS = -I. -I$(srcdir) -I../bfd -I$(srcdir)/config \
# How to link with both our special library facilities
# and the system's installed libraries.
-GASLIBS = @OPCODES_LIB@ ../bfd/libbfd.la ../libiberty/libiberty.a
+
+# FIXME:
+GASLIBS = @OPCODES_LIB@ ../bfd/libbfd.la ../libiberty/libiberty.a /home/david/coding-3/gcc-newgit-canvas-2023/build/gcc/libdiagnostics.so
# Files to be copied away after each stage in building.
STAGESTUFF = *.@OBJEXT@ $(noinst_PROGRAMS)
@@ -1300,6 +1300,7 @@ gas_early_init (int *argcp, char ***argvp)
static void
gas_init (void)
{
+ messages_init ();
symbol_begin ();
frag_init ();
subsegs_begin ();
@@ -1486,6 +1487,8 @@ main (int argc, char ** argv)
input_scrub_end ();
+ messages_end ();
+
/* Use xexit instead of return, because under VMS environments they
may not place the same interpretation on the value given. */
if (had_errors () != 0)
@@ -474,6 +474,9 @@ void as_bad_value_out_of_range (const char *, offsetT, offsetT, offsetT,
void print_version_id (void);
char * app_push (void);
+void messages_init (void);
+void messages_end (void);
+
/* Number of littlenums required to hold an extended precision number. */
#define MAX_LITTLENUMS 6
@@ -21,6 +21,14 @@
#include <limits.h>
#include <signal.h>
+// FIXME: do some configury thing for this
+#define USE_LIBDIAGNOSTICS 1
+
+#if USE_LIBDIAGNOSTICS
+// FIXME: need to fix this path, obviously
+#include "/home/david/coding-3/gcc-newgit-canvas-2023/src/gcc/libdiagnostics.h"
+#endif
+
/* If the system doesn't provide strsignal, we get it defined in
libiberty but no declaration is supplied. Because, reasons. */
#if !defined (HAVE_STRSIGNAL) && !defined (strsignal)
@@ -101,6 +109,29 @@ had_warnings (void)
return warning_count;
}
+#if USE_LIBDIAGNOSTICS
+static diagnostic_manager *diag_mgr;
+#endif
+
+void messages_init (void)
+{
+#if USE_LIBDIAGNOSTICS
+ diag_mgr = diagnostic_manager_new ();
+ diagnostic_manager_add_text_sink (diag_mgr, stderr,
+ DIAGNOSTIC_COLORIZE_IF_TTY);
+ diagnostic_manager_add_sarif_sink (diag_mgr, stderr,
+ DIAGNOSTIC_SARIF_VERSION_2_1_0);
+#endif
+}
+
+void messages_end (void)
+{
+#if USE_LIBDIAGNOSTICS
+ diagnostic_manager_release (diag_mgr);
+ diag_mgr = NULL;
+#endif
+}
+
/* Nonzero if we've hit a 'bad error', and should not write an obj file,
and exit with a nonzero error code. */
@@ -182,6 +213,20 @@ as_warn_internal (const char *file, unsigned int line, char *buffer)
context = true;
}
+#if USE_LIBDIAGNOSTICS
+ const diagnostic_file *file_obj
+ = diagnostic_manager_new_file (diag_mgr, file, NULL);
+
+ const diagnostic_physical_location *loc
+ = diagnostic_manager_new_location_from_file_and_line (diag_mgr,
+ file_obj,
+ line);
+
+ diagnostic *d = diagnostic_begin (diag_mgr,
+ DIAGNOSTIC_LEVEL_WARNING);
+ diagnostic_set_location (d, loc);
+ diagnostic_finish (d, "%s", buffer);
+#else
identify (file);
if (file)
{
@@ -192,6 +237,7 @@ as_warn_internal (const char *file, unsigned int line, char *buffer)
}
else
fprintf (stderr, "%s%s\n", _("Warning: "), buffer);
+#endif /* #else clause of #if USE_LIBDIAGNOSTICS */
if (context)
as_report_context ();
@@ -256,6 +302,19 @@ as_bad_internal (const char *file, unsigned int line, char *buffer)
context = true;
}
+#if USE_LIBDIAGNOSTICS
+ const diagnostic_file *file_obj
+ = diagnostic_manager_new_file (diag_mgr, file, NULL);
+ const diagnostic_physical_location *loc
+ = diagnostic_manager_new_location_from_file_and_line (diag_mgr,
+ file_obj,
+ line);
+
+ diagnostic *d = diagnostic_begin (diag_mgr,
+ DIAGNOSTIC_LEVEL_ERROR);
+ diagnostic_set_location (d, loc);
+ diagnostic_finish (d, "%s", buffer);
+#else
identify (file);
if (file)
{
@@ -266,6 +325,7 @@ as_bad_internal (const char *file, unsigned int line, char *buffer)
}
else
fprintf (stderr, "%s%s\n", _("Error: "), buffer);
+#endif /* #else clause of #if USE_LIBDIAGNOSTICS */
if (context)
as_report_context ();