Skip to content
Snippets Groups Projects
  • David Malcolm's avatar
    d495ea2b
    diagnostics: ensure that .sarif files are UTF-8 encoded [PR109098] · d495ea2b
    David Malcolm authored
    
    PR analyzer/109098 notes that the SARIF spec mandates that .sarif
    files are UTF-8 encoded, but -fdiagnostics-format=sarif-file naively
    assumes that the source files are UTF-8 encoded when quoting source
    artefacts in the .sarif output, which can lead to us writing out
    .sarif files with non-UTF-8 bytes in them (which break my reporting
    scripts).
    
    The root cause is that sarif_builder::maybe_make_artifact_content_object
    was using maybe_read_file to load the file content as bytes, and
    assuming they were UTF-8 encoded.
    
    This patch reworks both overloads of this function (one used for the
    whole file, the other for snippets of quoted lines) so that they go
    through input.cc's file cache, which attempts to decode the input files
    according to the input charset, and then encode as UTF-8.  They also
    check that the result actually is UTF-8, for cases where the input
    charset is missing, or incorrectly specified, and omit the quoted
    source for such awkward cases.
    
    Doing so fixes all of the cases I've encountered.
    
    The patch adds a new:
      { dg-final { verify-sarif-file } }
    directive to all SARIF test cases in the test suite, which verifies
    that the output is UTF-8 encoded, and is valid JSON.  In particular
    it verifies that when we complain about encoding problems, the .sarif
    report we emit is itself correctly encoded.
    
    gcc/ChangeLog:
    	PR analyzer/109098
    	* diagnostic-format-sarif.cc (read_until_eof): Delete.
    	(maybe_read_file): Delete.
    	(sarif_builder::maybe_make_artifact_content_object): Use
    	get_source_file_content rather than maybe_read_file.
    	Reject it if it's not valid UTF-8.
    	* input.cc (file_cache_slot::get_full_file_content): New.
    	(get_source_file_content): New.
    	(selftest::check_cpp_valid_utf8_p): New.
    	(selftest::test_cpp_valid_utf8_p): New.
    	(selftest::input_cc_tests): Call selftest::test_cpp_valid_utf8_p.
    	* input.h (get_source_file_content): New prototype.
    
    gcc/testsuite/ChangeLog:
    	PR analyzer/109098
    	* c-c++-common/diagnostic-format-sarif-file-1.c: Add
    	verify-sarif-file directive.
    	* c-c++-common/diagnostic-format-sarif-file-2.c: Likewise.
    	* c-c++-common/diagnostic-format-sarif-file-3.c: Likewise.
    	* c-c++-common/diagnostic-format-sarif-file-4.c: Likewise.
    	* c-c++-common/diagnostic-format-sarif-file-Wbidi-chars.c: New
    	test case, adapted from Wbidi-chars-1.c.
    	* c-c++-common/diagnostic-format-sarif-file-bad-utf8-pr109098-1.c:
    	New test case.
    	* c-c++-common/diagnostic-format-sarif-file-bad-utf8-pr109098-2.c:
    	New test case.
    	* c-c++-common/diagnostic-format-sarif-file-bad-utf8-pr109098-3.c:
    	New test case, adapted from cpp/Winvalid-utf8-1.c.
    	* c-c++-common/diagnostic-format-sarif-file-valid-CP850.c: New
    	test case, adapted from gcc.dg/diagnostic-input-charset-1.c.
    	* gcc.dg/plugin/crash-test-ice-sarif.c: Add verify-sarif-file
    	directive.
    	* gcc.dg/plugin/crash-test-write-though-null-sarif.c: Likewise.
    	* gcc.dg/plugin/diagnostic-test-paths-5.c: Likewise.
    	* lib/scansarif.exp (verify-sarif-file): New procedure.
    	* lib/verify-sarif-file.py: New support script.
    
    libcpp/ChangeLog:
    	PR analyzer/109098
    	* charset.cc (cpp_valid_utf8_p): New function.
    	* include/cpplib.h (cpp_valid_utf8_p): New prototype.
    
    Signed-off-by: default avatarDavid Malcolm <dmalcolm@redhat.com>
    d495ea2b
    History
    diagnostics: ensure that .sarif files are UTF-8 encoded [PR109098]
    David Malcolm authored
    
    PR analyzer/109098 notes that the SARIF spec mandates that .sarif
    files are UTF-8 encoded, but -fdiagnostics-format=sarif-file naively
    assumes that the source files are UTF-8 encoded when quoting source
    artefacts in the .sarif output, which can lead to us writing out
    .sarif files with non-UTF-8 bytes in them (which break my reporting
    scripts).
    
    The root cause is that sarif_builder::maybe_make_artifact_content_object
    was using maybe_read_file to load the file content as bytes, and
    assuming they were UTF-8 encoded.
    
    This patch reworks both overloads of this function (one used for the
    whole file, the other for snippets of quoted lines) so that they go
    through input.cc's file cache, which attempts to decode the input files
    according to the input charset, and then encode as UTF-8.  They also
    check that the result actually is UTF-8, for cases where the input
    charset is missing, or incorrectly specified, and omit the quoted
    source for such awkward cases.
    
    Doing so fixes all of the cases I've encountered.
    
    The patch adds a new:
      { dg-final { verify-sarif-file } }
    directive to all SARIF test cases in the test suite, which verifies
    that the output is UTF-8 encoded, and is valid JSON.  In particular
    it verifies that when we complain about encoding problems, the .sarif
    report we emit is itself correctly encoded.
    
    gcc/ChangeLog:
    	PR analyzer/109098
    	* diagnostic-format-sarif.cc (read_until_eof): Delete.
    	(maybe_read_file): Delete.
    	(sarif_builder::maybe_make_artifact_content_object): Use
    	get_source_file_content rather than maybe_read_file.
    	Reject it if it's not valid UTF-8.
    	* input.cc (file_cache_slot::get_full_file_content): New.
    	(get_source_file_content): New.
    	(selftest::check_cpp_valid_utf8_p): New.
    	(selftest::test_cpp_valid_utf8_p): New.
    	(selftest::input_cc_tests): Call selftest::test_cpp_valid_utf8_p.
    	* input.h (get_source_file_content): New prototype.
    
    gcc/testsuite/ChangeLog:
    	PR analyzer/109098
    	* c-c++-common/diagnostic-format-sarif-file-1.c: Add
    	verify-sarif-file directive.
    	* c-c++-common/diagnostic-format-sarif-file-2.c: Likewise.
    	* c-c++-common/diagnostic-format-sarif-file-3.c: Likewise.
    	* c-c++-common/diagnostic-format-sarif-file-4.c: Likewise.
    	* c-c++-common/diagnostic-format-sarif-file-Wbidi-chars.c: New
    	test case, adapted from Wbidi-chars-1.c.
    	* c-c++-common/diagnostic-format-sarif-file-bad-utf8-pr109098-1.c:
    	New test case.
    	* c-c++-common/diagnostic-format-sarif-file-bad-utf8-pr109098-2.c:
    	New test case.
    	* c-c++-common/diagnostic-format-sarif-file-bad-utf8-pr109098-3.c:
    	New test case, adapted from cpp/Winvalid-utf8-1.c.
    	* c-c++-common/diagnostic-format-sarif-file-valid-CP850.c: New
    	test case, adapted from gcc.dg/diagnostic-input-charset-1.c.
    	* gcc.dg/plugin/crash-test-ice-sarif.c: Add verify-sarif-file
    	directive.
    	* gcc.dg/plugin/crash-test-write-though-null-sarif.c: Likewise.
    	* gcc.dg/plugin/diagnostic-test-paths-5.c: Likewise.
    	* lib/scansarif.exp (verify-sarif-file): New procedure.
    	* lib/verify-sarif-file.py: New support script.
    
    libcpp/ChangeLog:
    	PR analyzer/109098
    	* charset.cc (cpp_valid_utf8_p): New function.
    	* include/cpplib.h (cpp_valid_utf8_p): New prototype.
    
    Signed-off-by: default avatarDavid Malcolm <dmalcolm@redhat.com>