Re: [PATCH] libstdc++: fix pointer type exception catch [PR105387]

[Jonathan Wakely <jwakely.gcc@gmail.com>]

On Tue, 3 May 2022 at 11:57, Jakob Hasse via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> This is a patch for the bug 105387 reported in bugzilla: 105387 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105387. This report should contain all the necessary information about the issue. But the patch there was just a preliminary one.
> I created a proper patch with the fix and a test case, based on TAG releases/gcc-11.2.0 (7ca388565af176bd4efd4f8db1e5e9e11e98ef45). The changelog is part of the commit message in the patch.

Thanks for the patch!

Some boring administrative comments:

In the summary line of the git commit message:
- The component should be libstdc++ not libstdc++-v3.
- The PR number should include "PR" i.e. [PR105387].
Your email Subject: gets this right, but the patch doesn't.
This is (not very well) documented at
https://gcc.gnu.org/contribute.html#patches

Your new testcase has a FSF copyright notice. Unless you have already
completed the paperwork to assign your work (either for just this
change, or this and all future changes) to the FSF then you need to
either do that legal paperwork, or alternatively contribute under the
DCO terms without assigning copyright. See
https://gcc.gnu.org/contribute.html#legal
For simplicity and to expedite the process, I suggest just removing
the copyright notice and license notice from the testcase, and adding
a Signed-off-by: tag to the commit message, as per
https://gcc.gnu.org/dco.html

As for the actual code ...

The testcase has unused parameters for the 'main' function:
+int main(int argc, char **argv)
That should be just 'int main()' because the parameters aren't used.

The testcase should mention the PR, e.g.
// PR libstdc++/105387

I'd prefer if the test contains the original reproducer from the PR,
which doesn't rely on internal details like __pointer_type_info. You
can put the portable test from the PR into one function and the
non-portable test into another function, and call them both from
main().

I'd prefer using unsigned char for the buffer, rather than uint8_t
(which requires the <stdint.h> header, and might be a typedef for
'int' on 8-bit targets, rather than a character type).

The test uses nullptr which is not available in C++98. It should
either require c++11 so it's UNSUPPORTED for c++98 mode:
// { dg-do run { target c++11 } }
or just use 0 instead of nullptr.



> Number of unexpected failures went down by one and the number of unsupported tests went  up by one. 17_intro/headers/c++1998/49745.cc suddenly passes, 22_locale/time_get/get_date/wchar_t/4.cc is suddenly unsupported. I don't know why.

I don't understand that either. I'll try to reproduce that.