In our .sarif output from e.g.:
bad-binary-op.c: In function ‘test_4’:
bad-binary-op.c:19:23: error: invalid operands to binary + (have ‘S’ {aka
‘struct s’} and ‘T’ {aka ‘struct t’})
19 | return callee_4a () + callee_4b ();
| ~~~~~~~~~~~~ ^ ~~~~~~~~~~~~
| | |
| | T {aka struct t}
| S {aka struct s}
the labelled ranges are captured in the 'annotations' property of the
'location' object (§3.28.6).
However sarif-replay emits just:
In function 'test_4':
bad-binary-op.c:19:23: error: invalid operands to binary + (have ‘S’ {aka
‘struct s’} and ‘T’ {aka ‘struct t’}) [error]
19 | return callee_4a () + callee_4b ();
| ^
missing the labelled ranges.
This patch adds support to sarif-replay for the 'annotations' property;
with this patch we emit:
In function 'test_4':
bad-binary-op.c:19:23: error: invalid operands to binary + (have ‘S’ {aka
‘struct s’} and ‘T’ {aka ‘struct t’}) [error]
19 | return callee_4a () + callee_4b ();
| ~~~~~~~~~~~~ ^ ~~~~~~~~~~~~
| | |
| | T {aka struct t}
| S {aka struct s}
thus showing the labelled ranges.
Doing so requires adding a new entrypoint to libgdiagnostics:
diagnostic_physical_location_get_file
Given that we haven't yet released a stable version and that
sarif-replay is built together with libgdiagnostics I didn't
bother updating the ABI version.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r15-7561-ga1f63ea36e9c9f.
gcc/ChangeLog:
PR sarif-replay/118881
* doc/libgdiagnostics/topics/physical-locations.rst: Add
diagnostic_physical_location_get_file.
* libgdiagnostics++.h (physical_location::get_file): New wrapper.
(diagnostic::add_location): Likewise.
* libgdiagnostics.cc (diagnostic_manager::get_file_by_name): New.
(diagnostic_physical_location::get_file): New.
(diagnostic_physical_location_get_file): New.
* libgdiagnostics.h (diagnostic_physical_location_get_file): New.
* libgdiagnostics.map (diagnostic_physical_location_get_file): New.
* libsarifreplay.cc (class annotation): New.
(add_any_annotations): New.
(sarif_replayer::handle_result_obj): Collect vectors of
annotations in the calls to handle_location_object and apply them
to "err" and to "note" as appropriate.
(sarif_replayer::handle_thread_flow_location_object): Pass nullptr
for annotations.
(sarif_replayer::handle_location_object): Handle §3.28.6
"annotations" property, using it to populate a new
"out_annotations" param.
gcc/testsuite/ChangeLog:
PR sarif-replay/118881
* sarif-replay.dg/2.1.0-valid/3.28.6-annotations-1.sarif: New test.
Signed-off-by: David Malcolm <dmalc...@redhat.com>
---
.../topics/physical-locations.rst | 5 +
gcc/libgdiagnostics++.h | 19 ++++
gcc/libgdiagnostics.cc | 33 +++++++
gcc/libgdiagnostics.h | 6 ++
gcc/libgdiagnostics.map | 2 +
gcc/libsarifreplay.cc | 94 ++++++++++++++++++-
.../2.1.0-valid/3.28.6-annotations-1.sarif | 47 ++++++++++
7 files changed, 201 insertions(+), 5 deletions(-)
create mode 100644
gcc/testsuite/sarif-replay.dg/2.1.0-valid/3.28.6-annotations-1.sarif
diff --git a/gcc/doc/libgdiagnostics/topics/physical-locations.rst
b/gcc/doc/libgdiagnostics/topics/physical-locations.rst
index fec4a8f221b..099e27e9822 100644
--- a/gcc/doc/libgdiagnostics/topics/physical-locations.rst
+++ b/gcc/doc/libgdiagnostics/topics/physical-locations.rst
@@ -198,6 +198,11 @@ are at the parentheses.
TODO: example of output
+.. function:: diagnostic_file *diagnostic_physical_location_get_file (const
diagnostic_physical_location *physical_loc)
+
+ Get the :type:`diagnostic_file` associated with a given physical location.
+
+
Associating diagnostics with locations
**************************************
diff --git a/gcc/libgdiagnostics++.h b/gcc/libgdiagnostics++.h
index af75113678c..39477a0bc4a 100644
--- a/gcc/libgdiagnostics++.h
+++ b/gcc/libgdiagnostics++.h
@@ -93,6 +93,8 @@ public:
: m_inner (location)
{}
+ file get_file () const;
+
const diagnostic_physical_location *m_inner;
};
@@ -199,6 +201,9 @@ public:
void
set_location (physical_location loc);
+ void
+ add_location (physical_location loc);
+
void
add_location_with_label (physical_location loc,
const char *text);
@@ -372,6 +377,14 @@ file::set_buffered_content (const char *data, size_t sz)
diagnostic_file_set_buffered_content (m_inner, data, sz);
}
+// class physical_location
+
+inline file
+physical_location::get_file () const
+{
+ return file (diagnostic_physical_location_get_file (m_inner));
+}
+
// class execution_path
inline diagnostic_event_id
@@ -448,6 +461,12 @@ diagnostic::add_location_with_label (physical_location loc,
diagnostic_add_location_with_label (m_inner, loc.m_inner, text);
}
+inline void
+diagnostic::add_location (physical_location loc)
+{
+ diagnostic_add_location (m_inner, loc.m_inner);
+}
+
inline void
diagnostic::set_logical_location (logical_location loc)
{
diff --git a/gcc/libgdiagnostics.cc b/gcc/libgdiagnostics.cc
index 89a9b7fb1d8..d274283742f 100644
--- a/gcc/libgdiagnostics.cc
+++ b/gcc/libgdiagnostics.cc
@@ -147,6 +147,8 @@ struct diagnostic_physical_location
m_inner (inner)
{}
+ diagnostic_file *get_file () const;
+
diagnostic_manager *m_mgr;
location_t m_inner;
};
@@ -445,6 +447,14 @@ public:
return file;
}
+ diagnostic_file *
+ get_file_by_name (const char *name)
+ {
+ if (diagnostic_file **slot = m_str_to_file_map.get (name))
+ return *slot;
+ return nullptr;
+ }
+
const diagnostic_physical_location *
new_location_from_file_and_line (const diagnostic_file *file,
diagnostic_line_num_t line_num)
@@ -943,6 +953,18 @@ diagnostic_file::set_buffered_content (const char *buf,
size_t sz)
fc.add_buffered_content (m_name.get_str (), buf, sz);
}
+// struct diagnostic_physical_location
+
+diagnostic_file *
+diagnostic_physical_location::get_file () const
+{
+ m_mgr->set_line_table_global ();
+ const char *filename = LOCATION_FILE (m_inner);
+ if (!filename)
+ return nullptr;
+ return m_mgr->get_file_by_name (filename);
+}
+
/* class impl_diagnostic_client_data_hooks. */
const client_version_info *
@@ -1753,3 +1775,14 @@ diagnostic_finish_va (diagnostic *diag, const char
*gmsgid, va_list *args)
diag->get_manager ().emit (*diag, gmsgid, args);
delete diag;
}
+
+/* Public entrypoint. */
+
+diagnostic_file *
+diagnostic_physical_location_get_file (const diagnostic_physical_location
*physical_loc)
+{
+ if (!physical_loc)
+ return nullptr;
+
+ return physical_loc->get_file ();
+}
diff --git a/gcc/libgdiagnostics.h b/gcc/libgdiagnostics.h
index f2041476786..2ce0f4c99c8 100644
--- a/gcc/libgdiagnostics.h
+++ b/gcc/libgdiagnostics.h
@@ -686,6 +686,12 @@ diagnostic_finish_va (diagnostic *diag, const char *fmt,
va_list *args)
LIBGDIAGNOSTICS_PARAM_MUST_BE_NON_NULL (2)
LIBGDIAGNOSTICS_PARAM_GCC_FORMAT_STRING (2, 0);
+/* Get the diagnostic_file associated with PHYSICAL_LOC. */
+
+extern diagnostic_file *
+diagnostic_physical_location_get_file (const diagnostic_physical_location
*physical_loc)
+ LIBGDIAGNOSTICS_PARAM_CAN_BE_NULL(0);
+
/* DEFERRED:
- thread-safety
- plural forms
diff --git a/gcc/libgdiagnostics.map b/gcc/libgdiagnostics.map
index 995e684a74b..5958cfe3f12 100644
--- a/gcc/libgdiagnostics.map
+++ b/gcc/libgdiagnostics.map
@@ -69,5 +69,7 @@ LIBGDIAGNOSTICS_ABI_0
diagnostic_finish;
diagnostic_finish_va;
+ diagnostic_physical_location_get_file;
+
local: *;
};
diff --git a/gcc/libsarifreplay.cc b/gcc/libsarifreplay.cc
index 71f80797926..075dadba41f 100644
--- a/gcc/libsarifreplay.cc
+++ b/gcc/libsarifreplay.cc
@@ -199,6 +199,23 @@ struct string_property_value
ValueType m_value;
};
+/* A class for recording annotations seen in locations (§3.28.6) that
+ should be emitted as secondary locations on diagnostics. */
+
+class annotation
+{
+public:
+ annotation (libgdiagnostics::physical_location phys_loc,
+ label_text label)
+ : m_phys_loc (phys_loc),
+ m_label (std::move (label))
+ {
+ }
+
+ libgdiagnostics::physical_location m_phys_loc;
+ label_text m_label;
+};
+
class sarif_replayer
{
public:
@@ -287,7 +304,8 @@ private:
handle_location_object (const json::object &location_obj,
const json::object &run_obj,
libgdiagnostics::physical_location &out_physical_loc,
- libgdiagnostics::logical_location &out_logical_loc);
+ libgdiagnostics::logical_location &out_logical_loc,
+ std::vector<annotation> *out_annotations);
// "physicalLocation" object (§3.29)
enum status
@@ -962,6 +980,18 @@ sarif_replayer::get_level_from_level_str (const
json::string &level_str)
level_values, ARRAY_SIZE (level_values));
}
+static void
+add_any_annotations (libgdiagnostics::diagnostic &diag,
+ const std::vector<annotation> &annotations)
+{
+ for (auto &annotation : annotations)
+ if (annotation.m_label.get ())
+ diag.add_location_with_label (annotation.m_phys_loc,
+ annotation.m_label.get ());
+ else
+ diag.add_location (annotation.m_phys_loc);
+}
+
/* Process a result object (SARIF v2.1.0 section 3.27).
Known limitations:
- doesn't yet handle "ruleIndex" property (§3.27.6)
@@ -1025,6 +1055,7 @@ sarif_replayer::handle_result_obj (const json::object
&result_obj,
= get_required_property<json::array> (result_obj, locations_prop);
if (!locations_arr)
return status::err_invalid_sarif;
+ std::vector<annotation> annotations;
if (locations_arr->length () > 0)
{
/* Only look at the first, if there's more than one. */
@@ -1035,7 +1066,8 @@ sarif_replayer::handle_result_obj (const json::object
&result_obj,
return status::err_invalid_sarif;
enum status s = handle_location_object (*location_obj, run_obj,
physical_loc,
- logical_loc);
+ logical_loc,
+ &annotations);
if (s != status::ok)
return s;
}
@@ -1092,6 +1124,7 @@ sarif_replayer::handle_result_obj (const json::object
&result_obj,
}
err.set_location (physical_loc);
err.set_logical_location (logical_loc);
+ add_any_annotations (err, annotations);
if (path.m_inner)
err.take_execution_path (std::move (path));
err.finish ("%s", text.get ());
@@ -1112,9 +1145,11 @@ sarif_replayer::handle_result_obj (const json::object
&result_obj,
prop_related_locations);
if (!location_obj)
return status::err_invalid_sarif;
+ std::vector<annotation> annotations;
enum status s = handle_location_object (*location_obj, run_obj,
physical_loc,
- logical_loc);
+ logical_loc,
+ &annotations);
if (s != status::ok)
return s;
@@ -1136,6 +1171,7 @@ sarif_replayer::handle_result_obj (const json::object
&result_obj,
auto note (m_output_mgr.begin_diagnostic (DIAGNOSTIC_LEVEL_NOTE));
note.set_location (physical_loc);
note.set_logical_location (logical_loc);
+ add_any_annotations (note, annotations);
note.finish ("%s", text.get ());
}
}
@@ -1490,7 +1526,8 @@ handle_thread_flow_location_object (const json::object
&tflow_loc_obj,
{
/* location object (§3.28). */
enum status s = handle_location_object (*location_obj, run_obj,
- physical_loc, logical_loc);
+ physical_loc, logical_loc,
+ nullptr);
if (s != status::ok)
return s;
@@ -1564,7 +1601,8 @@ sarif_replayer::
handle_location_object (const json::object &location_obj,
const json::object &run_obj,
libgdiagnostics::physical_location &out_physical_loc,
- libgdiagnostics::logical_location &out_logical_loc)
+ libgdiagnostics::logical_location &out_logical_loc,
+ std::vector<annotation> *out_annotations)
{
// §3.28.3 "physicalLocation" property
{
@@ -1604,6 +1642,52 @@ handle_location_object (const json::object &location_obj,
}
}
+ // §3.28.6 "annotations" property
+ {
+ const property_spec_ref annotations_prop
+ ("location", "annotations", "3.28.6");
+ if (const json::array *annotations_arr
+ = get_optional_property<json::array> (location_obj,
+ annotations_prop))
+ for (auto element : *annotations_arr)
+ {
+ const json::object *annotation_obj
+ = require_object_for_element (*element, annotations_prop);
+ if (!annotation_obj)
+ return status::err_invalid_sarif;
+ libgdiagnostics::file file = out_physical_loc.get_file ();
+ if (!file.m_inner)
+ return report_invalid_sarif
+ (*annotation_obj, annotations_prop,
+ "cannot find artifact for %qs property",
+ annotations_prop.get_property_name ());
+ libgdiagnostics::physical_location phys_loc;
+ enum status s = handle_region_object (*annotation_obj,
+ file,
+ phys_loc);
+ if (s != status::ok)
+ return s;
+
+ label_text label;
+
+ // §3.30.14 message property
+ {
+ const property_spec_ref message_prop
+ ("region", "message", "3.30.14");
+
+ if (const json::object *message_obj
+ = get_optional_property<json::object> (*annotation_obj,
+ message_prop))
+ label = make_plain_text_within_result_message (nullptr,
+ *message_obj,
+ nullptr);
+ }
+
+ if (out_annotations)
+ out_annotations->push_back ({phys_loc, std::move (label)});
+ }
+ }
+
return status::ok;
}
diff --git
a/gcc/testsuite/sarif-replay.dg/2.1.0-valid/3.28.6-annotations-1.sarif
b/gcc/testsuite/sarif-replay.dg/2.1.0-valid/3.28.6-annotations-1.sarif
new file mode 100644
index 00000000000..4ff6e07ab4d
--- /dev/null
+++ b/gcc/testsuite/sarif-replay.dg/2.1.0-valid/3.28.6-annotations-1.sarif
@@ -0,0 +1,47 @@
+{"$schema":
"https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/schemas/sarif-schema-2.1.0.json";,
+ "version": "2.1.0",
+ "runs": [{"tool": {"driver": {"name": "GNU C23",
+ "fullName": "GNU C23 (GCC) version 15.0.1
20250203 (experimental) (x86_64-pc-linux-gnu)",
+ "version": "15.0.1 20250203 (experimental)",
+ "informationUri": "https://gcc.gnu.org/gcc-15/";,
+ "rules": []}},
+ "invocations": [{"executionSuccessful": false,
+ "toolExecutionNotifications": []}],
+ "artifacts": [{"location": {"uri":
"bad-binary-ops-highlight-colors.c"},
+ "sourceLanguage": "c",
+ "contents": {"text": "/* Verify that colorization
affects both text within diagnostic messages\n and underlined ranges of
quoted source, and that the types we use\n match up between them.\n Also
implicitly verify that -fdiagnostics-show-highlight-colors is\n on by
default. */\n\n\n\nstruct s {};\nstruct t {};\ntypedef struct s S;\ntypedef
struct t T;\n\nextern S callee_4a (void);\nextern T callee_4b (void);\n\nint
test_4 (void)\n{\n return callee_4a () + callee_4b ();\n}\n"},
+ "roles": ["analysisTarget"]}],
+ "results": [{"ruleId": "error",
+ "level": "error",
+ "message": {"text": "invalid operands to binary +
(have ‘S’ {{aka ‘struct s’}} and ‘T’ {{aka ‘struct t’}})"},
+ "locations": [{"physicalLocation":
{"artifactLocation": {"uri": "bad-binary-ops-highlight-colors.c",
+
"uriBaseId": "PWD"},
+ "region":
{"startLine": 19,
+
"startColumn": 23,
+
"endColumn": 24},
+ "contextRegion":
{"startLine": 19,
+
"snippet": {"text": " return callee_4a () + callee_4b ();\n"}}},
+ "logicalLocations": [{"name": "test_4",
+
"fullyQualifiedName": "test_4",
+ "decoratedName":
"test_4",
+ "kind":
"function"}],
+ "annotations": [{"startLine": 19,
+ "startColumn": 10,
+ "endColumn": 22,
+ "message": {"text": "S
{{aka struct s}}"}},
+ {"startLine": 19,
+ "startColumn": 25,
+ "endColumn": 37,
+ "message": {"text": "T
{{aka struct t}}"}}]}]}]}]}
+
+/* Verify that we underline and label the ranges for the
+ "annotations" above. */
+/* { dg-begin-multiline-output "" }
+bad-binary-ops-highlight-colors.c:19:23: error: invalid operands to binary +
(have ‘S’ {aka ‘struct s’} and ‘T’ {aka ‘struct t’}) [error]
+ 19 | return callee_4a () + callee_4b ();
+ | ~~~~~~~~~~~~ ^ ~~~~~~~~~~~~
+ | | |
+ | | T {aka struct t}
+ | S {aka struct s}
+ { dg-end-multiline-output "" } */
+// TODO: trailing [error]
--
2.26.3
