https://gcc.gnu.org/g:24b6d2014035073870d9d8dae9152fc16fc319fd
commit r15-8447-g24b6d2014035073870d9d8dae9152fc16fc319fd
Author: David Malcolm <dmalc...@redhat.com>
Date: Wed Mar 19 15:03:42 2025 -0400
diagnostics: fix crash in urlifier with -Wfatal-errors [PR119366]
diagnostic_context's dtor assumed that it owned the m_urlifier pointer
and would delete it.
As of r15-5988-g5a022062d22e0b this isn't always the case -
auto_urlify_attributes is used in various places in the C/C++ frontends
and in the middle-end to temporarily override the urlifier with an
on-stack instance, which would lead to delete-of-on-stack-buffer crashes
with -Wfatal-errors as the global_dc was cleaned up.
Fix by explicitly tracking the stack of urlifiers within
diagnostic_context, tracking for each node whether the pointer is
owned or borrowed.
gcc/ChangeLog:
PR c/119366
* diagnostic-format-sarif.cc (test_message_with_embedded_link):
Convert diagnostic_context from one urlifier to a stack of
urlifiers, where each node in the stack tracks whether the
urlifier is owned or borrowed.
* diagnostic.cc (diagnostic_context::initialize): Likewise.
(diagnostic_context::finish): Likewise.
(diagnostic_context::set_urlifier): Delete.
(diagnostic_context::push_owned_urlifier): New.
(diagnostic_context::push_borrowed_urlifier): New.
(diagnostic_context::pop_urlifier): New.
(diagnostic_context::get_urlifier): Reimplement in terms of stack.
(diagnostic_context::override_urlifier): Delete.
* diagnostic.h (diagnostic_context::set_urlifier): Delete decl.
(diagnostic_context::override_urlifier): Delete decl.
(diagnostic_context::push_owned_urlifier): New decl.
(diagnostic_context::push_borrowed_urlifier): New decl.
(diagnostic_context::pop_urlifier): New decl.
(diagnostic_context::get_urlifier): Make return value const; hide
implementation.
(diagnostic_context::m_urlifier): Replace with...
(diagnostic_context::urlifier_stack_node): ... this and...
(diagnostic_context::m_urlifier_stack): ...this.
* gcc-urlifier.cc
(auto_override_urlifier::auto_override_urlifier): Reimplement.
(auto_override_urlifier::~auto_override_urlifier): Reimplement.
* gcc-urlifier.h (class auto_override_urlifier): Reimplement.
(auto_urlify_attributes::auto_urlify_attributes): Update for
pass-by-reference.
* gcc.cc (driver::global_initializations): Update for
reimplementation of urlifiers in terms of a stack.
* toplev.cc (general_init): Likewise.
gcc/testsuite/ChangeLog:
PR c/119366
* gcc.dg/Wfatal-bad-attr-pr119366.c: New test.
Signed-off-by: David Malcolm <dmalc...@redhat.com>
Diff:
---
gcc/diagnostic-format-sarif.cc | 2 +-
gcc/diagnostic.cc | 57 ++++++++++++++++++-------
gcc/diagnostic.h | 23 +++++++---
gcc/gcc-urlifier.cc | 7 ++-
gcc/gcc-urlifier.h | 7 +--
gcc/gcc.cc | 2 +-
gcc/testsuite/gcc.dg/Wfatal-bad-attr-pr119366.c | 8 ++++
gcc/toplev.cc | 2 +-
8 files changed, 74 insertions(+), 34 deletions(-)
diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-format-sarif.cc
index 554992bddba1..8dbc91ee8f32 100644
--- a/gcc/diagnostic-format-sarif.cc
+++ b/gcc/diagnostic-format-sarif.cc
@@ -4365,7 +4365,7 @@ test_message_with_embedded_link (enum sarif_version
version)
};
test_sarif_diagnostic_context dc ("test.c", version);
- dc.set_urlifier (::make_unique<test_urlifier> ());
+ dc.push_owned_urlifier (::make_unique<test_urlifier> ());
rich_location richloc (line_table, UNKNOWN_LOCATION);
dc.report (DK_ERROR, richloc, nullptr, 0,
"foo %<-foption%> %<unrecognized%> bar");
diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index c2f6714c24aa..82d7f946818b 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -254,7 +254,7 @@ diagnostic_context::initialize (int n_opts)
m_text_callbacks.m_start_span = default_diagnostic_start_span_fn;
m_text_callbacks.m_end_diagnostic = default_diagnostic_text_finalizer;
m_option_mgr = nullptr;
- m_urlifier = nullptr;
+ m_urlifier_stack = new auto_vec<urlifier_stack_node> ();
m_last_location = UNKNOWN_LOCATION;
m_client_aux_data = nullptr;
m_lock = 0;
@@ -424,8 +424,13 @@ diagnostic_context::finish ()
delete m_option_mgr;
m_option_mgr = nullptr;
- delete m_urlifier;
- m_urlifier = nullptr;
+ if (m_urlifier_stack)
+ {
+ while (!m_urlifier_stack->is_empty ())
+ pop_urlifier ();
+ delete m_urlifier_stack;
+ m_urlifier_stack = nullptr;
+ }
freeargv (m_original_argv);
m_original_argv = nullptr;
@@ -549,13 +554,43 @@ set_option_manager
(std::unique_ptr<diagnostic_option_manager> mgr,
}
void
-diagnostic_context::set_urlifier (std::unique_ptr<urlifier> urlifier)
+diagnostic_context::push_owned_urlifier (std::unique_ptr<urlifier> ptr)
{
- delete m_urlifier;
- /* Ideally the field would be a std::unique_ptr here. */
- m_urlifier = urlifier.release ();
+ gcc_assert (m_urlifier_stack);
+ const urlifier_stack_node node = { ptr.release (), true };
+ m_urlifier_stack->safe_push (node);
}
+void
+diagnostic_context::push_borrowed_urlifier (const urlifier &loan)
+{
+ gcc_assert (m_urlifier_stack);
+ const urlifier_stack_node node = { const_cast <urlifier *> (&loan), false };
+ m_urlifier_stack->safe_push (node);
+}
+
+void
+diagnostic_context::pop_urlifier ()
+{
+ gcc_assert (m_urlifier_stack);
+ gcc_assert (m_urlifier_stack->length () > 0);
+
+ const urlifier_stack_node node = m_urlifier_stack->pop ();
+ if (node.m_owned)
+ delete node.m_urlifier;
+}
+
+const urlifier *
+diagnostic_context::get_urlifier () const
+{
+ if (!m_urlifier_stack)
+ return nullptr;
+ if (m_urlifier_stack->is_empty ())
+ return nullptr;
+ return m_urlifier_stack->last ().m_urlifier;
+}
+
+
/* Set PP as the reference printer for this context.
Refresh all output sinks. */
@@ -605,14 +640,6 @@ diagnostic_context::set_prefixing_rule
(diagnostic_prefixing_rule_t rule)
pp_prefixing_rule (sink->get_printer ()) = rule;
}
-/* Set the urlifier without deleting the existing one. */
-
-void
-diagnostic_context::override_urlifier (urlifier *urlifier)
-{
- m_urlifier = urlifier;
-}
-
void
diagnostic_context::create_edit_context ()
{
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 202760b2f85d..62bffd2c6851 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -609,8 +609,11 @@ public:
void set_output_format (std::unique_ptr<diagnostic_output_format>
output_format);
void set_text_art_charset (enum diagnostic_text_art_charset charset);
void set_client_data_hooks (std::unique_ptr<diagnostic_client_data_hooks>
hooks);
- void set_urlifier (std::unique_ptr<urlifier>);
- void override_urlifier (urlifier *);
+
+ void push_owned_urlifier (std::unique_ptr<urlifier>);
+ void push_borrowed_urlifier (const urlifier &);
+ void pop_urlifier ();
+
void create_edit_context ();
void set_warning_as_error_requested (bool val)
{
@@ -667,7 +670,8 @@ public:
{
return m_client_data_hooks;
}
- urlifier *get_urlifier () const { return m_urlifier; }
+
+ const urlifier *get_urlifier () const;
text_art::theme *get_diagram_theme () const { return m_diagrams.m_theme; }
@@ -888,11 +892,16 @@ private:
diagnostic_option_manager *m_option_mgr;
unsigned m_lang_mask;
- /* An optional hook for adding URLs to quoted text strings in
+ /* A stack of optional hooks for adding URLs to quoted text strings in
diagnostics. Only used for the main diagnostic message.
- Owned by the context; this would be a std::unique_ptr if
- diagnostic_context had a proper ctor. */
- urlifier *m_urlifier;
+ Typically a single one owner by the context, but can be temporarily
+ overridden by a borrowed urlifier (e.g. on-stack). */
+ struct urlifier_stack_node
+ {
+ urlifier *m_urlifier;
+ bool m_owned;
+ };
+ auto_vec<urlifier_stack_node> *m_urlifier_stack;
public:
/* Auxiliary data for client. */
diff --git a/gcc/gcc-urlifier.cc b/gcc/gcc-urlifier.cc
index 49611b71d9b9..a4094582f416 100644
--- a/gcc/gcc-urlifier.cc
+++ b/gcc/gcc-urlifier.cc
@@ -220,15 +220,14 @@ make_gcc_urlifier (unsigned int lang_mask)
/* class auto_override_urlifier. */
-auto_override_urlifier::auto_override_urlifier (urlifier *new_urlifier)
-: m_old_urlifier (global_dc->get_urlifier ())
+auto_override_urlifier::auto_override_urlifier (const urlifier &new_urlifier)
{
- global_dc->override_urlifier (new_urlifier);
+ global_dc->push_borrowed_urlifier (new_urlifier);
}
auto_override_urlifier::~auto_override_urlifier ()
{
- global_dc->override_urlifier (m_old_urlifier);
+ global_dc->pop_urlifier ();
}
#if CHECKING_P
diff --git a/gcc/gcc-urlifier.h b/gcc/gcc-urlifier.h
index 5ffbbf7a4ceb..eefed4903282 100644
--- a/gcc/gcc-urlifier.h
+++ b/gcc/gcc-urlifier.h
@@ -33,11 +33,8 @@ extern char *make_doc_url (const char *doc_url_suffix);
class auto_override_urlifier
{
public:
- auto_override_urlifier (urlifier *new_urlifier);
+ auto_override_urlifier (const urlifier &new_urlifier);
~auto_override_urlifier ();
-
-protected:
- urlifier * const m_old_urlifier;
};
/* Subclass of urlifier that attempts to add URLs to quoted strings
@@ -71,7 +68,7 @@ class auto_urlify_attributes
{
public:
auto_urlify_attributes ()
- : m_override (&m_urlifier)
+ : m_override (m_urlifier)
{
}
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index 04b3736a5da1..c7b2aa6df166 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -8356,7 +8356,7 @@ driver::global_initializations ()
diagnostic_initialize (global_dc, 0);
diagnostic_color_init (global_dc);
diagnostic_urls_init (global_dc);
- global_dc->set_urlifier (make_gcc_urlifier (0));
+ global_dc->push_owned_urlifier (make_gcc_urlifier (0));
#ifdef GCC_DRIVER_HOST_INITIALIZATION
/* Perform host dependent initialization when needed. */
diff --git a/gcc/testsuite/gcc.dg/Wfatal-bad-attr-pr119366.c
b/gcc/testsuite/gcc.dg/Wfatal-bad-attr-pr119366.c
new file mode 100644
index 000000000000..2ca4eedadeda
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wfatal-bad-attr-pr119366.c
@@ -0,0 +1,8 @@
+/* Verify that we don't crash if we bail out with a fatal error
+ while an on-stack attribute_urlifier is active (PR c/119366). */
+/* { dg-options "-Wfatal-errors -Werror=attributes" } */
+void foo() __attribute__((this_does_not_exist)); /* { dg-error
"'this_does_not_exist' attribute directive ignored" } */
+
+/* { dg-message "some warnings being treated as errors" "treated as errors"
{target "*-*-*"} 0 } */
+/* { dg-message "terminated due to -Wfatal-errors" "terminated" { target *-*-*
} 0 } */
+
diff --git a/gcc/toplev.cc b/gcc/toplev.cc
index c5d46ec98015..6d8b8852fb84 100644
--- a/gcc/toplev.cc
+++ b/gcc/toplev.cc
@@ -1099,7 +1099,7 @@ general_init (const char *argv0, bool init_signals,
unique_argv original_argv)
lang_mask,
&global_options),
lang_mask);
- global_dc->set_urlifier (make_gcc_urlifier (lang_mask));
+ global_dc->push_owned_urlifier (make_gcc_urlifier (lang_mask));
if (init_signals)
{
