diff mbox series

libstdc++: Add test for LWG Issue 3897

Message ID 20231204164231.784822-1-hawkinsw@obs.cr
State Committed
Headers
Series libstdc++: Add test for LWG Issue 3897 |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Will Hawkins Dec. 4, 2023, 4:42 p.m. UTC 
  Hello!

Thank you, as always, for the great work that you do on libstdc++. The
inout_ptr implementation properly handles the issue raised in LWG 3897
but it seems like having an explicit test might be a good idea.

I hope that this helps!
Will

-- >8 --

Add a test to verify that the implementation of inout_ptr is not
vulnerable to LWG Issue 3897.

libstdc++-v3/ChangeLog:

	* testsuite/20_util/smartptr.adapt/inout_ptr/3.cc: New test
	for LWG Issue 3897.

Signed-off-by: Will Hawkins <hawkinsw@obs.cr>
---
 .../20_util/smartptr.adapt/inout_ptr/3.cc       | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/3.cc

Comments

Jonathan Wakely Dec. 5, 2023, 3:45 p.m. UTC | #1 
On Mon, 4 Dec 2023 at 16:42, Will Hawkins wrote:
>
> Hello!
>
> Thank you, as always, for the great work that you do on libstdc++. The
> inout_ptr implementation properly handles the issue raised in LWG 3897
> but it seems like having an explicit test might be a good idea.

Thanks, Will, we should definitely have a test for this.

I've tweaked it a bit to avoid leaking the pointer (in case anybody
runs the tests under valgrind or ASan) and to add your new test to the
existing file (to avoid the overhead of a separate test just for this
one check).

See attached ...
commit c02f3696fdb07d1a06c1aa7b035be9a20d65b803
Author: Will Hawkins <hawkinsw@obs.cr>
Date:   Mon Dec 4 20:59:44 2023

    libstdc++: Add test for LWG Issue 3897
    
    Add a test to verify that the implementation of inout_ptr is not
    vulnerable to LWG Issue 3897.
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/20_util/smartptr.adapt/inout_ptr/2.cc: Add check
            for LWG Issue 3897.
    
    Co-authored-by: Jonathan Wakely <jwakely@redhat.com>

diff --git a/libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/2.cc b/libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/2.cc
index ca6076209c2..b4a2d95227a 100644
--- a/libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/2.cc
+++ b/libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/2.cc
@@ -96,7 +96,22 @@ test_unique_ptr()
   VERIFY( upbd->id == 2 );
 }
 
+void
+test_lwg3897()
+{
+  // Verify that implementation handles LWG Issue 3897
+  auto nuller = [](int** p) {
+    delete *p;
+    *p = nullptr;
+  };
+  int* i = new int{5};
+  nuller(std::inout_ptr(i));
+
+  VERIFY( i == nullptr );
+}
+
 int main()
 {
   test_unique_ptr();
+  test_lwg3897();
 }
Will Hawkins Dec. 5, 2023, 3:49 p.m. UTC | #2 
On Tue, Dec 5, 2023 at 10:46 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Mon, 4 Dec 2023 at 16:42, Will Hawkins wrote:
> >
> > Hello!
> >
> > Thank you, as always, for the great work that you do on libstdc++. The
> > inout_ptr implementation properly handles the issue raised in LWG 3897
> > but it seems like having an explicit test might be a good idea.
>
> Thanks, Will, we should definitely have a test for this.
>
> I've tweaked it a bit to avoid leaking the pointer (in case anybody
> runs the tests under valgrind or ASan) and to add your new test to the

Of course ... how could I forget to delete the pointer? I'm a goofball.

> existing file (to avoid the overhead of a separate test just for this
> one check).

Makes perfect sense. I wasn't sure how you typically handle that. I
will know for the future.

>
> See attached ...

Thank you for the feedback! I look forward to the next time I can help!
Will
Jonathan Wakely Dec. 5, 2023, 4:32 p.m. UTC | #3 
On Tue, 5 Dec 2023 at 15:57, Will Hawkins wrote:
>
> On Tue, Dec 5, 2023 at 10:46 AM Jonathan Wakely <jwakely@redhat.com> wrote:
> >
> > On Mon, 4 Dec 2023 at 16:42, Will Hawkins wrote:
> > >
> > > Hello!
> > >
> > > Thank you, as always, for the great work that you do on libstdc++. The
> > > inout_ptr implementation properly handles the issue raised in LWG 3897
> > > but it seems like having an explicit test might be a good idea.
> >
> > Thanks, Will, we should definitely have a test for this.
> >
> > I've tweaked it a bit to avoid leaking the pointer (in case anybody
> > runs the tests under valgrind or ASan) and to add your new test to the
>
> Of course ... how could I forget to delete the pointer? I'm a goofball.

:-)

> > existing file (to avoid the overhead of a separate test just for this
> > one check).
>
> Makes perfect sense. I wasn't sure how you typically handle that. I
> will know for the future.

In principle it's better to have one test file per thing we want to
check ... but libstdc++ has a lot of tests, and every one of them
includes the bits/stdc++.h precompiled header which includes the
entire library. And the way dejagnu works, every test runs multiple
compilations, because it preprocesses or compiles various helper files
to check the test conditions. And since I run every test about 20
times (with various combinations of options) it all adds up.
diff mbox series

Patch

diff --git a/libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/3.cc b/libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/3.cc
new file mode 100644
index 00000000000..f9114dc57b5
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/smartptr.adapt/inout_ptr/3.cc
@@ -0,0 +1,17 @@ 
+// { dg-do run { target c++23 } }
+
+#include <memory>
+#include <testsuite_hooks.h>
+
+// C++23 [inout.ptr.t] Class template inout_ptr_t
+// Verify that implementation handles LWG Issue 3897
+void nuller(int **p) {
+  *p = nullptr;
+}
+
+int main(int, char **) {
+  int *i = new int{5};
+  nuller(std::inout_ptr(i));
+
+  VERIFY(i == nullptr);
+}