Segmentation fault with const string& constructor and templated std::set find

Question

The following code encountered a segment fault with the const string& constructor, and exit with 0 with the string_view constructor. I know the const string& is not the best way to do so. But from my understanding, without optimization, a temp string is constructed from const char*, then it's value gets copied in the Person constructor before it gets destroyed. And the constructor looks valid to me. For the find part, the set::find (string_view) will use the defined less operators as needed. And I didn't see what's wrong here. I am able to reproduce the issue with g++-10 or VS2022.

#include <set>
#include <iostream>
#include <string>
#include <string_view>
using namespace std;

struct Person{
    string name;
    Person(const string& s):name{s}{}
    //Person(string_view s):name{s}{}
    bool operator<(string_view s)const{
        return name<s;
    }
    bool operator<(const Person& o)const{
        return name<o.name;
    }
};
bool operator<(string_view s,const Person& p){
    return s<p.name;
}
set<Person,less<>>s;

int main(){
    s.emplace("hello");
    string_view hel="hel";
    auto res=s.find(hel);
}

gdb gives me the following information about the crash:

Program received signal SIGSEGV, Segmentation fault.
0x000000000800280c in std::char_traits<char>::copy (__s1=0x7fffff7ef1c0 "\340\361~\377\377\177", __s2=0x7fffff7ef230 "hello",
    __n=<error reading variable: Cannot access memory at address 0x7fffff7eeff8>) at /usr/include/c++/10/bits/char_traits.h:401
401           copy(char_type* __s1, const char_type* __s2, size_t __n)

If you try to see the reason for the crash in the debugger, the problem appears to be quite obvious (hint: look at the call stack). Have you tried using your debugger? This is what it's there for. — Sam Varshavchik
– Sam Varshavchik, Commented Sep 5, 2021 at 1:03
@SamVarshavchik thanks for the suggestion, previously I only considered if the arguments look valid or not, when I do a backtrace it's obvious. — James
– James, Commented Sep 5, 2021 at 1:30

Adrian Mole · Accepted Answer · 2021-09-05 01:26:46Z

3

Your free-standing operator< calls itself recursively (and infinitely). This is because there is no in-built < operator that compares a string_view to a string, yet there is an implicit conversion from string to const Person&.

Although there is also an implicit conversion from string to string_view, because there is an available conversion in the scope/namespace of the function itself, that is chosen over any other, found by argument-dependent lookup for the two operand types (thanks to Sam Varshavchik for pointing this out).

So, in the body of that function, the right-hand operand of the < comparison is converted to a const Person&, and the function thus calls itself.

To fix the issue, convert the operands to the same STL type; either to 2 string_view operands, like this:

bool operator<(string_view s, const Person& p)
{
    return s < string_view(p.name);
}

or to 2 string operands, like this:

bool operator<(string_view s, const Person& p)
{
    return string(s) < p.name;
}

Note: This is not a particularly easy type of problem to spot. However, turning on (full) compiler warnings will likely help. For your original code, MSVC gives me this:

warning C4717: 'operator<': recursive on all control paths, function will cause runtime stack overflow

And clang shows:

warning : all paths through this function will call itself [-Winfinite-recursion]

edited Sep 5, 2021 at 1:26

answered Sep 5, 2021 at 1:06

Adrian Mole

52.1k193 gold badges61 silver badges101 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

James Over a year ago

wow, since it compiled, I didn't realize there wasn't a < operator for string_view and string

Sam Varshavchik Over a year ago

There's also an implicit conversion from std::string to a std::string_view. A complete answer would include why this implicit conversion was chosen.

Adrian Mole Over a year ago

@SamVarshavchik Thinking about it. Care to offer some help? :-) (It's very late where I am and way past the time I should be in the Land of Nod ... I'll sleep on it and try to offer a formal explanation.)

Sam Varshavchik Over a year ago

I believe that this is because implicit conversions in the current namespace are preferred over the ones found via argument-dependent lookups.

Adrian Mole Over a year ago

Thanks. I was working on the lines of ADL in the scope (namespace) of the function itself. I guess the function itself matches so there's is no further ADL.

Collectives™ on Stack Overflow

Segmentation fault with const string& constructor and templated std::set find

1 Answer 1

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related