Bug 91878 - No sanitizer report for past-end access of std∷set
Summary: No sanitizer report for past-end access of std∷set
Status: RESOLVED WONTFIX
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 9.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-24 08:54 UTC by Konstantin Kharlamov
Modified: 2019-09-24 09:51 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-09-24 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Kharlamov 2019-09-24 08:54:11 UTC
# Steps to reproduce (in terms of terminal commands)

    $ cat test2.cpp
    #include <set>
    #include <cstdio>

    int main() {
        std::set<int> s{};
        printf("%i\n", *s.begin());
    }
    $ g++ test2.cpp -o a -g3 -O0 -fsanitize=address,undefined
    $ ./a

## Expected

It either crashes on past-end access, or produces a warning (depending on whether it's handled as a past-end access or UB).

## Actual

It prints:

    0
Comment 1 Konstantin Kharlamov 2019-09-24 08:57:28 UTC
Btw, worth noting that clang 8.0.1 does not handle it either.
Comment 2 Martin Liška 2019-09-24 09:17:22 UTC
Confirmed, we're slowly adding instrumentation for std containers to sanitizers.
Comment 3 Marc Glisse 2019-09-24 09:31:53 UTC
-D_GLIBCXX_DEBUG is the current way to add many checks to libstdc++, and it detects this.
Comment 4 Konstantin Kharlamov 2019-09-24 09:33:45 UTC
(In reply to Marc Glisse from comment #3)
> -D_GLIBCXX_DEBUG is the current way to add many checks to libstdc++, and it
> detects this.

Oh, cool, I'll make use of it, thanks for the hint!
Comment 5 Jonathan Wakely 2019-09-24 09:36:48 UTC
(In reply to Konstantin Kharlamov from comment #0)
> It either crashes on past-end access, or produces a warning (depending on
> whether it's handled as a past-end access or UB).

No, that's not how undefined behaviour works. You are wrong to expect a crash, and not all cases of undefined behaviour can be detected reliably.

I don't think this case can be caught by sanitizers. As Marc said (and as I suggested on your previous bug report) you should use -D_GLIBCXX_DEBUG to catch these kind of bugs.
Comment 6 Konstantin Kharlamov 2019-09-24 09:44:39 UTC
(In reply to Jonathan Wakely from comment #5)

> No, that's not how undefined behaviour works. You are wrong to expect a crash

No, in context of the report I'm not. You're correct this is not how UB works, but this is how address sanitizer does.

> and not all cases of undefined behaviour can be detected reliably.

Well, given -D_GLIBCXX_DEBUG does handle it, probably sanitizer can either.

> As Marc said (and as I suggested on your previous bug report) you should use
> -D_GLIBCXX_DEBUG to catch these kind of bugs.

Right, thanks, FTR: my prev. report was handled by sanitizer correctly, so back then I wouldn't need to use the additional debugging option.
Comment 7 Konstantin Kharlamov 2019-09-24 09:47:22 UTC
@Jonathan Wakely I think you accidentally closed the report, would you mind to reopen it (I'm not seeing why would it be "invalid", people even confirmed that more support for std containers is being added to sanitizers).
Comment 8 Jonathan Wakely 2019-09-24 09:51:18 UTC
(In reply to Konstantin Kharlamov from comment #6)
> (In reply to Jonathan Wakely from comment #5)
> 
> > No, that's not how undefined behaviour works. You are wrong to expect a crash
> 
> No, in context of the report I'm not. You're correct this is not how UB
> works, but this is how address sanitizer does.

The past-the-end iterator of a std::set doesn't point to the heap, and points to a valid object that is always readable. I don't see how asan can help here.

> > and not all cases of undefined behaviour can be detected reliably.
> 
> Well, given -D_GLIBCXX_DEBUG does handle it, probably sanitizer can either.

No, I don't agree. They work very differently.
 
Closing the bug was not an accident, I don't believe there are any plans to add asan instrumentation to std::set, and I don't believe it's possible to handle the past-the-end case.
Comment 9 Jonathan Wakely 2019-09-24 09:51:53 UTC
I'll call it WONTFIX rather than INVALID if you prefer, but it's not going to be handled by asan either way.