On Fri, Oct 4, 2024 at 12:04 PM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> The PR notes that the new pch_save/pch_restore methods I've added
> recently invoke UB if either m_classification_history.address ()
> or m_push_list.address () is NULL (which can happen if those vectors
> are empty (and in the pch_save case nothing has been pushed into them
> before either). While the corresponding length is necessarily 0,
> fwrite (NULL, something, 0, f) or
> fread (NULL, something, 0, f) still invoke UB.
>
> The following patch fixes that by not calling fwrite/fread if the
> corresponding length is 0.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
> 2024-10-04 Jakub Jelinek <ja...@redhat.com>
>
> PR pch/116936
> * diagnostic.cc (diagnostic_option_classifier::pch_save): Only call
> fwrite if corresponding length is non-zero.
> (diagnostic_option_classifier::pch_restore): Only call fread if
> corresponding length is non-zero.
>
> --- gcc/diagnostic.cc.jj 2024-10-01 09:38:58.014961851 +0200
> +++ gcc/diagnostic.cc 2024-10-02 20:33:37.922953272 +0200
> @@ -165,11 +165,13 @@ diagnostic_option_classifier::pch_save (
> unsigned int lengths[2] = { m_classification_history.length (),
> m_push_list.length () };
> if (fwrite (lengths, sizeof (lengths), 1, f) != 1
> - || fwrite (m_classification_history.address (),
> - sizeof (diagnostic_classification_change_t),
> - lengths[0], f) != lengths[0]
> - || fwrite (m_push_list.address (), sizeof (int),
> - lengths[1], f) != lengths[1])
> + || (lengths[0]
> + && fwrite (m_classification_history.address (),
> + sizeof (diagnostic_classification_change_t),
> + lengths[0], f) != lengths[0])
> + || (lengths[1]
> + && fwrite (m_push_list.address (), sizeof (int),
> + lengths[1], f) != lengths[1]))
> return -1;
> return 0;
> }
> @@ -187,11 +189,13 @@ diagnostic_option_classifier::pch_restor
> gcc_checking_assert (m_push_list.is_empty ());
> m_classification_history.safe_grow (lengths[0]);
> m_push_list.safe_grow (lengths[1]);
> - if (fread (m_classification_history.address (),
> - sizeof (diagnostic_classification_change_t),
> - lengths[0], f) != lengths[0]
> - || fread (m_push_list.address (), sizeof (int),
> - lengths[1], f) != lengths[1])
> + if ((lengths[0]
> + && fread (m_classification_history.address (),
> + sizeof (diagnostic_classification_change_t),
> + lengths[0], f) != lengths[0])
> + || (lengths[1]
> + && fread (m_push_list.address (), sizeof (int),
> + lengths[1], f) != lengths[1]))
> return -1;
> return 0;
> }
>
> Jakub
>
