[Bug c++/91777] New: No warning for iterator going out of scope

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug c++/91777] New: No warning for iterator going out of scope

seurer at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91777

            Bug ID: 91777
           Summary: No warning for iterator going out of scope
           Product: gcc
           Version: 9.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: Hi-Angel at yandex dot ru
  Target Milestone: ---

The testcase below would've worked if instead of `return {l,…` was used `return
{move(l),…`. But it wasn't, and due to lack of borrow-semantics in C++ language
such mistakes are easy to make. In these circumstances programmers has to rely
on compiler warnings, so it would be amazing if GCC warn about iterators still
referencing the out-of-scope container.

# Steps to reproduce (in terms of terminal commands)

    $ cat test.cpp
    #include <cstdio>
    #include <list>

    using namespace std;

    pair<list<int>, list<int>::const_iterator> foo() {
        list<int> l = {1,2,3};
        return {l, l.begin()};
    }

    int main() {
        pair p = foo();
        printf("%i\n", *p.second);
    }
    $ g++ test.cpp -o a -O3 -Wall -Wextra -Wsign-conversion -std=c++17
-fsanitize=address,undefined

## Expected

A warning about the second element in `return {l, l.begin()}` become invalid
once `l` goes out of scope.

## Actual

No output.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/91777] No warning for iterator going out of scope

seurer at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91777

--- Comment #1 from Konstantin Kharlamov <Hi-Angel at yandex dot ru> ---
FTR, on IRC was referenced the following paper that may be interesting for
implementors
https://github.com/isocpp/CppCoreGuidelines/blob/master/docs/Lifetime.pdf
Reply | Threaded
Open this post in threaded view
|

[Bug c++/91777] No warning for iterator going out of scope

seurer at gcc dot gnu.org
In reply to this post by seurer at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91777

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2019-09-16
                 CC|                            |marxin at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
I can see a ASAN error:

$ g++ pr91777.cc -std=c++17 -fsanitize=address -g -O3 -Wall -Wextra
-Wsign-conversion && ./a.out
=================================================================
==21067==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000000020
at pc 0x000000401464 bp 0x7ffe073683b0 sp 0x7ffe073683a8
READ of size 4 at 0x603000000020 thread T0
    #0 0x401463 in main /home/marxin/Programming/testcases/pr91777.cc:13
    #1 0x7fd31dd8cbca in __libc_start_main ../csu/libc-start.c:308
    #2 0x401529 in _start (/home/marxin/Programming/testcases/a.out+0x401529)

0x603000000020 is located 16 bytes inside of 24-byte region
[0x603000000010,0x603000000028)
freed by thread T0 here:
    #0 0x7fd31e374f37 in operator delete(void*, unsigned long)
/home/marxin/Programming/gcc/libsanitizer/asan/asan_new_delete.cpp:172
    #1 0x401c7f in __gnu_cxx::new_allocator<std::_List_node<int>
>::deallocate(std::_List_node<int>*, unsigned long)
/home/marxin/bin/gcc/include/c++/10.0.0/ext/new_allocator.h:129
    #2 0x401c7f in std::allocator_traits<std::allocator<std::_List_node<int> >
>::deallocate(std::allocator<std::_List_node<int> >&, std::_List_node<int>*,
unsigned long) /home/marxin/bin/gcc/include/c++/10.0.0/bits/alloc_traits.h:470
    #3 0x401c7f in std::__cxx11::_List_base<int, std::allocator<int>
>::_M_put_node(std::_List_node<int>*)
/home/marxin/bin/gcc/include/c++/10.0.0/bits/stl_list.h:442
    #4 0x401c7f in std::__cxx11::_List_base<int, std::allocator<int>
>::_M_clear() /home/marxin/bin/gcc/include/c++/10.0.0/bits/list.tcc:81
    #5 0x401c7f in std::__cxx11::_List_base<int, std::allocator<int>
>::~_List_base() /home/marxin/bin/gcc/include/c++/10.0.0/bits/stl_list.h:495
    #6 0x401c7f in std::__cxx11::list<int, std::allocator<int> >::~list()
/home/marxin/bin/gcc/include/c++/10.0.0/bits/stl_list.h:823
    #7 0x401c7f in foo[abi:cxx11]()
/home/marxin/Programming/testcases/pr91777.cc:7
    #8 0x60300000006f  (<unknown module>)

previously allocated by thread T0 here:
    #0 0x7fd31e374117 in operator new(unsigned long)
/home/marxin/Programming/gcc/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x40184c in __gnu_cxx::new_allocator<std::_List_node<int>
>::allocate(unsigned long, void const*)
/home/marxin/bin/gcc/include/c++/10.0.0/ext/new_allocator.h:111
    #2 0x40184c in std::allocator_traits<std::allocator<std::_List_node<int> >
>::allocate(std::allocator<std::_List_node<int> >&, unsigned long)
/home/marxin/bin/gcc/include/c++/10.0.0/bits/alloc_traits.h:444
    #3 0x40184c in std::__cxx11::_List_base<int, std::allocator<int>
>::_M_get_node() /home/marxin/bin/gcc/include/c++/10.0.0/bits/stl_list.h:438
    #4 0x40184c in std::_List_node<int>* std::__cxx11::list<int,
std::allocator<int> >::_M_create_node<int const&>(int const&)
/home/marxin/bin/gcc/include/c++/10.0.0/bits/stl_list.h:630
    #5 0x40184c in void std::__cxx11::list<int, std::allocator<int>
>::_M_insert<int const&>(std::_List_iterator<int>, int const&)
/home/marxin/bin/gcc/include/c++/10.0.0/bits/stl_list.h:1907
    #6 0x40184c in int& std::__cxx11::list<int, std::allocator<int>
>::emplace_back<int const&>(int const&)
/home/marxin/bin/gcc/include/c++/10.0.0/bits/stl_list.h:1223
    #7 0x40184c in void std::__cxx11::list<int, std::allocator<int>
>::_M_initialize_dispatch<int const*>(int const*, int const*,
std::__false_type) /home/marxin/bin/gcc/include/c++/10.0.0/bits/stl_list.h:1836
    #8 0x40184c in std::__cxx11::list<int, std::allocator<int>
>::list(std::initializer_list<int>, std::allocator<int> const&)
/home/marxin/bin/gcc/include/c++/10.0.0/bits/stl_list.h:757
    #9 0x40184c in foo[abi:cxx11]()
/home/marxin/Programming/testcases/pr91777.cc:7
    #10 0x7fd300000002  (<unknown module>)

SUMMARY: AddressSanitizer: heap-use-after-free
/home/marxin/Programming/testcases/pr91777.cc:13 in main
Shadow bytes around the buggy address:
  0x0c067fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c067fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c067fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c067fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c067fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c067fff8000: fa fa fd fd[fd]fa fa fa fd fd fd fa fa fa fd fd
  0x0c067fff8010: fd fa fa fa 00 00 00 fa fa fa 00 00 00 fa fa fa
  0x0c067fff8020: 00 00 00 fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==21067==ABORTING
Reply | Threaded
Open this post in threaded view
|

[Bug c++/91777] No warning for iterator going out of scope

seurer at gcc dot gnu.org
In reply to this post by seurer at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91777

--- Comment #3 from Konstantin Kharlamov <Hi-Angel at yandex dot ru> ---
(In reply to Martin Liška from comment #2)
> I can see a ASAN error:

Correct. You set the report status to WAITING: do you expect some answer from
me, or was it accidental?
Reply | Threaded
Open this post in threaded view
|

[Bug c++/91777] No warning for iterator going out of scope

seurer at gcc dot gnu.org
In reply to this post by seurer at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91777

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Konstantin Kharlamov from comment #3)
> (In reply to Martin Liška from comment #2)
> > I can see a ASAN error:
>
> Correct. You set the report status to WAITING: do you expect some answer
> from me, or was it accidental?

Well, you mentioned that there's no output from ASAN. But I see it properly
reported. That's why I set the status to waiting.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/91777] No warning for iterator going out of scope

seurer at gcc dot gnu.org
In reply to this post by seurer at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91777

--- Comment #5 from Konstantin Kharlamov <Hi-Angel at yandex dot ru> ---
(In reply to Martin Liška from comment #4)
> (In reply to Konstantin Kharlamov from comment #3)
> > (In reply to Martin Liška from comment #2)
> > > I can see a ASAN error:
> >
> > Correct. You set the report status to WAITING: do you expect some answer
> > from me, or was it accidental?
>
> Well, you mentioned that there's no output from ASAN. But I see it properly
> reported. That's why I set the status to waiting.

You misread it :) I didn't mention asan, and "steps to reproduce" lack the step
of running the executable.

I did add the sanitizer to options in the "steps" though, just to make sure if
anyone tries to execute the binary, it gonna crash.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/91777] No warning for iterator going out of scope

seurer at gcc dot gnu.org
In reply to this post by seurer at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91777

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So here is the thing.  Tracking things via memory is hard.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/91777] No warning for iterator going out of scope

seurer at gcc dot gnu.org
In reply to this post by seurer at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91777

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It's also diagnosed by libstdc++ Debug Mode:

/home/jwakely/gcc/10/include/c++/10.0.0/debug/safe_iterator.h:294:
In function:
    __gnu_debug::_Safe_iterator<_Iterator, _Sequence, _Category>::reference
    __gnu_debug::_Safe_iterator<_Iterator, _Sequence,
    _Category>::operator*() const [with _Iterator =
    std::__cxx1998::_List_const_iterator<int>; _Sequence =
    std::__debug::list<int>; _Category = std::forward_iterator_tag;
    __gnu_debug::_Safe_iterator<_Iterator, _Sequence, _Category>::reference
    = const int&]

Error: attempt to dereference a singular iterator.

Objects involved in the operation:
    iterator "this" @ 0x0x7ffffb1db7f0 {
      type = std::__cxx1998::_List_const_iterator<int> (constant iterator);
      state = singular;
    }
Aborted (core dumped)


I don't think it's feasible to warn about this. As far as the compiler knows,
the iterator is just a value type. It's not practical to expect the compiler to
track that it contains a pointer to a node that is about to be destroyed by a
container going out of scope.