Re: [PATCH] binutils: experimental use of libdiagnostics in gas

[Jan Beulich <jbeulich@suse.com>]

On 06.11.2023 23:29, David Malcolm wrote:
> 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": ""}}]}]}]}
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> which I see:
> - drops the leading "Assembler messages" warning,
> - changes the capitalization of the "Warning" -> "warning" etc
> - quotes the pertinent line in the .s file
> 
> All of the locations are just lines; does gas do column numbers at all?
> (or ranges?)

It currently doesn't, which is primarily related to the scrubbing done
before lines are actually processed.

I take it that the lack of column information is why there are lines of
this form

      |

in the example output above. Them uniformly not carrying any information
would make it desirable for them to be suppressed.

> @@ -172,16 +203,34 @@ as_tsktsk (const char *format, ...)
>  static void
>  as_warn_internal (const char *file, unsigned int line, char *buffer)
>  {
> +#if !USE_LIBDIAGNOSTICS
>    bool context = false;
> +#endif
>  
>    ++warning_count;
>  
>    if (file == NULL)
>      {
>        file = as_where_top (&line);
> +#if !USE_LIBDIAGNOSTICS
>        context = true;
> +#endif

I can't spot how this context information would be replaced. It works
for macros only right now, but the hope is to eventually extend it
also to .include files.

> @@ -199,6 +248,7 @@ as_warn_internal (const char *file, unsigned int line, char *buffer)
>  #ifndef NO_LISTING
>    listing_warning (buffer);
>  #endif
> +#endif /* #else clause of #if USE_LIBDIAGNOSTICS */

This listing integration of course needs to remain irrespective of
which way of emitting diagnostics is used.

Jan