Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss
Why does this work in MSVC 2019...
43 views
Skip to first unread message
Chris M. Thomasson
May 17, 2021, 7:05:59 PM5/17/21
to
I don't think this should work at all! It should segfault.
_______________________________
#include <iostream>
#include <algorithm>
#include <string>
#include <set>
struct person
{
person(std::string const& name, unsigned long id)
: m_name(name), m_id(id)
{
std::cout << "(" << this << ")->person::person(" << name << ",
" << id << ")\n";
}
~person()
{
std::cout << "(" << this << ")->person::~person(" << m_name <<
", " << m_id << ")\n";
}
void display() const
{
std::cout << "(" << this << ")->person::display(" << m_name <<
", " << m_id << ")\n";
}
std::string m_name;
unsigned long m_id;
};
struct database
{
struct key_id
{
using is_transparent = void; // Humm...
bool operator() (person const& p0, person const& p1) const
{
return p0.m_id < p1.m_id;
}
bool operator() (std::string const name, person const& p) const
{
return name < p.m_name;
}
bool operator() (person const& p, std::string const name) const
{
return p.m_name < name;
}
bool operator() (unsigned long id, person const& p) const
{
return id < p.m_id;
}
bool operator() (person const& p, unsigned long id) const
{
return p.m_id < id;
}
};
typedef std::set<person, key_id> set_t;
void add_entry(person const& p0)
{
m_set.insert(p0);
}
set_t::iterator const& search_id(unsigned long id) const
{
return m_set.find(id);
}
set_t::iterator const& search_id(std::string const& name) const
{
return m_set.find(name);
}
void display() const
{
std::cout << "\n(" << this << ")->database::display()\n";
std::cout << "_____________________________________\n";
std::for_each(m_set.begin(), m_set.end(), [](person const& p)
{
p.display();
});
std::cout << "_____________________________________\n\n";
}
set_t m_set;
};
int main()
{
{
database db;
db.add_entry(person("Chris", 42));
db.add_entry(person("Lisa", 23));
db.add_entry(person("Randall", 37));
std::cout << "_____________________________________\n\n";
{
database::set_t::iterator result = db.search_id(37);
if (result != db.m_set.end())
{
std::cout << "FOUND 37!\n";
(*result).display();
}
}
{
database::set_t::iterator result = db.search_id(137);
if (result == db.m_set.end()) std::cout << "137 NOT
FOUND!\n\n";
}
{
database::set_t::iterator result = db.search_id("Lisa");
if (result != db.m_set.end())
{
std::cout << "FOUND ""Lisa""\n";
(*result).display();
}
}
db.display();
}
return 0;
}
_______________________________
_______________________________
#include <iostream>
#include <algorithm>
#include <string>
#include <set>
struct person
{
person(std::string const& name, unsigned long id)
: m_name(name), m_id(id)
{
std::cout << "(" << this << ")->person::person(" << name << ",
" << id << ")\n";
}
~person()
{
std::cout << "(" << this << ")->person::~person(" << m_name <<
", " << m_id << ")\n";
}
void display() const
{
std::cout << "(" << this << ")->person::display(" << m_name <<
", " << m_id << ")\n";
}
std::string m_name;
unsigned long m_id;
};
struct database
{
struct key_id
{
using is_transparent = void; // Humm...
bool operator() (person const& p0, person const& p1) const
{
return p0.m_id < p1.m_id;
}
bool operator() (std::string const name, person const& p) const
{
return name < p.m_name;
}
bool operator() (person const& p, std::string const name) const
{
return p.m_name < name;
}
bool operator() (unsigned long id, person const& p) const
{
return id < p.m_id;
}
bool operator() (person const& p, unsigned long id) const
{
return p.m_id < id;
}
};
typedef std::set<person, key_id> set_t;
void add_entry(person const& p0)
{
m_set.insert(p0);
}
set_t::iterator const& search_id(unsigned long id) const
{
return m_set.find(id);
}
set_t::iterator const& search_id(std::string const& name) const
{
return m_set.find(name);
}
void display() const
{
std::cout << "\n(" << this << ")->database::display()\n";
std::cout << "_____________________________________\n";
std::for_each(m_set.begin(), m_set.end(), [](person const& p)
{
p.display();
});
std::cout << "_____________________________________\n\n";
}
set_t m_set;
};
int main()
{
{
database db;
db.add_entry(person("Chris", 42));
db.add_entry(person("Lisa", 23));
db.add_entry(person("Randall", 37));
std::cout << "_____________________________________\n\n";
{
database::set_t::iterator result = db.search_id(37);
if (result != db.m_set.end())
{
std::cout << "FOUND 37!\n";
(*result).display();
}
}
{
database::set_t::iterator result = db.search_id(137);
if (result == db.m_set.end()) std::cout << "137 NOT
FOUND!\n\n";
}
{
database::set_t::iterator result = db.search_id("Lisa");
if (result != db.m_set.end())
{
std::cout << "FOUND ""Lisa""\n";
(*result).display();
}
}
db.display();
}
return 0;
}
_______________________________
Chris M. Thomasson
May 17, 2021, 7:11:43 PM5/17/21
to
On 5/17/2021 4:05 PM, Chris M. Thomasson wrote:
> I don't think this should work at all! It should segfault.
> _______________________________
[snip buggy code that seems to work in MSVC]
> I don't think this should work at all! It should segfault.
> _______________________________
> _______________________________
>
Now, this version should go ahead and work? It seems to.
_________________________________
{
return m_set.find(id);
}
set_t::iterator const search_id(std::string const& name) const
{
return m_set.find(name);
}
void display() const
{
std::cout << "\n(" << this << ")->database::display()\n";
std::cout << "_____________________________________\n";
std::for_each(m_set.begin(), m_set.end(), [](person const& p)
{
p.display();
});
std::cout << "_____________________________________\n\n";
}
set_t m_set;
};
int main()
{
{
database db;
db.add_entry(person("Chris", 42));
db.add_entry(person("Lisa", 23));
db.add_entry(person("Randall", 37));
std::cout << "_____________________________________\n\n";
{
database::set_t::iterator const& result = db.search_id(37);
return m_set.find(name);
}
void display() const
{
std::cout << "\n(" << this << ")->database::display()\n";
std::cout << "_____________________________________\n";
std::for_each(m_set.begin(), m_set.end(), [](person const& p)
{
p.display();
});
std::cout << "_____________________________________\n\n";
}
set_t m_set;
};
int main()
{
{
database db;
db.add_entry(person("Chris", 42));
db.add_entry(person("Lisa", 23));
db.add_entry(person("Randall", 37));
std::cout << "_____________________________________\n\n";
{
if (result != db.m_set.end())
{
std::cout << "FOUND 37!\n";
(*result).display();
}
}
{
database::set_t::iterator const& result = db.search_id(137);
{
std::cout << "FOUND 37!\n";
(*result).display();
}
}
{
if (result == db.m_set.end()) std::cout << "137 NOT
FOUND!\n\n";
}
{
database::set_t::iterator const& result = db.search_id("Lisa");
FOUND!\n\n";
}
{
if (result != db.m_set.end())
{
std::cout << "FOUND ""Lisa""\n";
(*result).display();
}
}
db.display();
}
return 0;
}
_________________________________
{
std::cout << "FOUND ""Lisa""\n";
(*result).display();
}
}
db.display();
}
return 0;
}
Branimir Maksimovic
May 17, 2021, 7:24:17 PM5/17/21
to
On 2021-05-17, Chris M. Thomasson <chris.m.t...@gmail.com> wrote:
> On 5/17/2021 4:05 PM, Chris M. Thomasson wrote:
>> I don't think this should work at all! It should segfault.
>> _______________________________
> [snip buggy code that seems to work in MSVC]
>> _______________________________
>>
>
> Now, this version should go ahead and work? It seems to.
Correcti, first version segfaults because temporary and I have warning,
> On 5/17/2021 4:05 PM, Chris M. Thomasson wrote:
>> I don't think this should work at all! It should segfault.
>> _______________________________
> [snip buggy code that seems to work in MSVC]
>> _______________________________
>>
>
> Now, this version should go ahead and work? It seems to.
second version does not has warning and works.
--
current job title: senior software engineer
skills: x86 aasembler,c++,c,rust,go,nim,haskell...
press any key to continue or any other to quit...
Mike Terry
May 18, 2021, 12:02:12 PM5/18/21
to
segfault?" with no further explanation isn't good. It would have been
helpful at least have said which bit of code you thought should produce
a segfault...
OK, so I guess your issue is with the two search_id functions, which
return a reference to a local result, e.g.:
> set_t::iterator const& search_id(unsigned long id) const
> {
> return m_set.find(id);
> }
17) there is no seg-fault because the original iterator storage on the
stack hasn't been overwritten at that point. (There is a compile
warning "returning address of local variable or temporary".)
The obvious correct approach would be to just return the iterator by
value. (And I'd wonder why would you want to make it const, but I
suppose this is some kind of experiment rather than production code.)
Mike.
Bonita Montero
May 18, 2021, 12:13:50 PM5/18/21
to
> > set_t::iterator const& search_id(unsigned long id) const
> > {
> > return m_set.find(id);
> > }
Totally brain-damaged.
> > {
> > return m_set.find(id);
> > }
Mike Terry
May 18, 2021, 12:19:20 PM5/18/21
to
by value (good). But now you're receiving them as references to a const
iterator, i.e. you're taking an rvalue result and extending its lifetime
by taking a const reference to it. That is allowed, but is bizarre in
this situation. I'm not sure why it would ever be good to do this, but
I've not thought very much about it. (If you were a beginner, I'd say
you should be just storing the value...)
Mike.
Alf P. Steinbach
May 18, 2021, 4:29:19 PM5/18/21
to
On 18.05.2021 01:05, Chris M. Thomasson wrote:
> I don't think this should work at all! It should segfault.
>
> I don't think this should work at all! It should segfault.
>
> set_t::iterator const& search_id(unsigned long id) const
Well UB is UB. That includes that things may appear to work.
- ALf
Chris M. Thomasson
May 18, 2021, 5:50:48 PM5/18/21
to
I was just wondering why MSVC allows this to run in the first place.
Chris M. Thomasson
May 18, 2021, 5:59:46 PM5/18/21
to
On 5/18/2021 9:01 AM, Mike Terry wrote:
> On 18/05/2021 00:05, Chris M. Thomasson wrote:
>> I don't think this should work at all! It should segfault.
>> _______________________________
[...]
> On 18/05/2021 00:05, Chris M. Thomasson wrote:
>> I don't think this should work at all! It should segfault.
>> _______________________________
>> _______________________________
>
> Posting around 150 lines of code, then asking "why doesn't this
> segfault?" with no further explanation isn't good. It would have been
> helpful at least have said which bit of code you thought should produce
> a segfault...
Yeah, sorry about that.
>
> Posting around 150 lines of code, then asking "why doesn't this
> segfault?" with no further explanation isn't good. It would have been
> helpful at least have said which bit of code you thought should produce
> a segfault...
> OK, so I guess your issue is with the two search_id functions, which
> return a reference to a local result, e.g.:
>
> > set_t::iterator const& search_id(unsigned long id) const
> > {
> > return m_set.find(id);
> > }
>
> When these are used you expect a segfault, but when I run the code (MSVS
> 17) there is no seg-fault because the original iterator storage on the
> stack hasn't been overwritten at that point. (There is a compile
> warning "returning address of local variable or temporary".)
>
> The obvious correct approach would be to just return the iterator by
> value. (And I'd wonder why would you want to make it const, but I
> suppose this is some kind of experiment rather than production code.)
I noticed that MSVC actually runs the bugged code. I got the warnings as
well, but still wondered about why it ran to completion.
Chris M. Thomasson
May 18, 2021, 6:06:08 PM5/18/21
to
On 5/18/2021 9:19 AM, Mike Terry wrote:
> On 18/05/2021 00:11, Chris M. Thomasson wrote:
>> On 5/17/2021 4:05 PM, Chris M. Thomasson wrote:
>>> I don't think this should work at all! It should segfault.
>>> _______________________________
>> [snip buggy code that seems to work in MSVC]
>>> _______________________________
>>>
>>
>> Now, this version should go ahead and work? It seems to.
>> _________________________________
[...]
> On 18/05/2021 00:11, Chris M. Thomasson wrote:
>> On 5/17/2021 4:05 PM, Chris M. Thomasson wrote:
>>> I don't think this should work at all! It should segfault.
>>> _______________________________
>> [snip buggy code that seems to work in MSVC]
>>> _______________________________
>>>
>>
>> Now, this version should go ahead and work? It seems to.
>> _________________________________
>> set_t::iterator const search_id(unsigned long id) const
>> {
>> return m_set.find(id);
>> }
>>
>> set_t::iterator const search_id(std::string const& name) const
>> {
>> return m_set.find(name);
>> }
[...]
>> {
>> return m_set.find(id);
>> }
>>
>> set_t::iterator const search_id(std::string const& name) const
>> {
>> return m_set.find(name);
>> }
>> _________________________________
>
> ..so now you've corrected the search_id() methods to return the iterator
> by value (good). But now you're receiving them as references to a const
> iterator, i.e. you're taking an rvalue result and extending its lifetime
> by taking a const reference to it. That is allowed, but is bizarre in
> this situation. I'm not sure why it would ever be good to do this, but
> I've not thought very much about it. (If you were a beginner, I'd say
> you should be just storing the value...)
Well, I was thinking that searching for something in the database using
>
> ..so now you've corrected the search_id() methods to return the iterator
> by value (good). But now you're receiving them as references to a const
> iterator, i.e. you're taking an rvalue result and extending its lifetime
> by taking a const reference to it. That is allowed, but is bizarre in
> this situation. I'm not sure why it would ever be good to do this, but
> I've not thought very much about it. (If you were a beginner, I'd say
> you should be just storing the value...)
a function qualified as const, should return a const iterator. So I just
added in the const keyword in vain. Its been a while since I used the
STL. Take mercy on my soul!
;^o
Chris M. Thomasson
May 18, 2021, 6:09:42 PM5/18/21
to
Chris M. Thomasson
May 18, 2021, 6:10:50 PM5/18/21
to
On 5/18/2021 9:12 AM, Bonita Montero wrote:
Bonita Montero
May 18, 2021, 7:43:01 PM5/18/21
to
>>> > set_t::iterator const& search_id(unsigned long id) const
>>> > {
>>> > return m_set.find(id);
>>> > }
>>
>> Totally brain-damaged.
> :^)
> I was just wondering why MSVC allows this to run in the first place.
Why not ? It's not uncompilable code.
>>> > {
>>> > return m_set.find(id);
>>> > }
>>
>> Totally brain-damaged.
> :^)
> I was just wondering why MSVC allows this to run in the first place.
Chris M. Thomasson
May 18, 2021, 7:49:13 PM5/18/21
to
MSVC, however it will not even run on GCC without a segfault. Plus, the
code has a bug. I get warnings in MSVC, but it still runs it to
completion. I was just wondering why.
Chris M. Thomasson
May 18, 2021, 8:43:59 PM5/18/21
to
Öö Tiib
May 18, 2021, 8:55:13 PM5/18/21
to
that the compiler segfaults (that i've not seen gcc to do for ages).
MSVC did allow to take lvalue references to non const of temporaries (that
should not compile) for quite long time. I have not checked if it has
stopped it finally ... that may have something to do with different outcome
of undefined behavior.
Undefined behavior is what it is ... different nasal demons or lack
of such from repeated runs of same compiled binary with same
data are rare but possible with MSVC and gcc alike.
Chris M. Thomasson
May 18, 2021, 11:06:59 PM5/18/21
to
On 5/18/2021 5:55 PM, Öö Tiib wrote:
> On Wednesday, 19 May 2021 at 02:49:13 UTC+3, Chris M. Thomasson wrote:
>> On 5/18/2021 4:42 PM, Bonita Montero wrote:
>>>>>>> set_t::iterator const& search_id(unsigned long id) const
>>>>>>> {
>>>>>>> return m_set.find(id);
>>>>>>> }
>>>>>
>>>>> Totally brain-damaged.
>>>
>>>> :^)
>>>> I was just wondering why MSVC allows this to run in the first place.
>>>
>>> Why not ? It's not uncompilable code.
>> Well, warnings aside for a moment, it seems to compile, _and_ run on
>> MSVC, however it will not even run on GCC without a segfault. Plus, the
>> code has a bug. I get warnings in MSVC, but it still runs it to
>> completion. I was just wondering why.
>
> The segfault indicates that gcc allows it to run as well unless you mean
> that the compiler segfaults (that i've not seen gcc to do for ages).
Yes, GCC allows it to run with warnings, and segfaults, and does not
> On Wednesday, 19 May 2021 at 02:49:13 UTC+3, Chris M. Thomasson wrote:
>> On 5/18/2021 4:42 PM, Bonita Montero wrote:
>>>>>>> set_t::iterator const& search_id(unsigned long id) const
>>>>>>> {
>>>>>>> return m_set.find(id);
>>>>>>> }
>>>>>
>>>>> Totally brain-damaged.
>>>
>>>> :^)
>>>> I was just wondering why MSVC allows this to run in the first place.
>>>
>>> Why not ? It's not uncompilable code.
>> Well, warnings aside for a moment, it seems to compile, _and_ run on
>> MSVC, however it will not even run on GCC without a segfault. Plus, the
>> code has a bug. I get warnings in MSVC, but it still runs it to
>> completion. I was just wondering why.
>
> The segfault indicates that gcc allows it to run as well unless you mean
> that the compiler segfaults (that i've not seen gcc to do for ages).
allow the bugged program to run to completion.
>
> MSVC did allow to take lvalue references to non const of temporaries (that
> should not compile) for quite long time. I have not checked if it has
> stopped it finally ... that may have something to do with different outcome
> of undefined behavior.
warnings, and lets the compiled program run to completion. Strange.
>
> Undefined behavior is what it is ... different nasal demons or lack
> of such from repeated runs of same compiled binary with same
> data are rare but possible with MSVC and gcc alike.
>
Bonita Montero
May 19, 2021, 1:02:31 AM5/19/21
to
> Well, warnings aside for a moment, it seems to compile, _and_ run
> on MSVC, however it will not even run on GCC without a segfault.
It's implementation-defined how long the memory of the temporary is
> on MSVC, however it will not even run on GCC without a segfault.
untouched.
Christian Gollwitzer
May 19, 2021, 1:57:30 AM5/19/21
to
Am 19.05.21 um 00:09 schrieb Chris M. Thomasson:
You are basically accessing memory that was "freed"[*] before. That is
totally undefined, of course, but there is no guarantee that it
segfaults. The CPU can not check every small allocation on the stack or
heap for correctness. Instead, the memory is structured into pages of
typically 4kB in size. The CPU only checks that the page was allocated
for the current process, so as long as there is any other valid object
in this page, there is no segfault. This also explains why programs with
memory errors often crash in a totally different location. If a pointer
gets mangled, then it segfaults at the point where the pointer is used
and suddenly points into the woods.
You can use a memory debugger to detect these problems, like valgrind on
Linux (one of the best) or the flag -fsanitize=address with clang. Then,
every object is allocated on its own page, making the program very
bloated and slow, but the program will then segfault immediately at the
point where the deleted object is accessed.
[*] not on the heap, so not "malloc/free-type freed", but "stack-freed"
Best regards,
Christian
totally undefined, of course, but there is no guarantee that it
segfaults. The CPU can not check every small allocation on the stack or
heap for correctness. Instead, the memory is structured into pages of
typically 4kB in size. The CPU only checks that the page was allocated
for the current process, so as long as there is any other valid object
in this page, there is no segfault. This also explains why programs with
memory errors often crash in a totally different location. If a pointer
gets mangled, then it segfaults at the point where the pointer is used
and suddenly points into the woods.
You can use a memory debugger to detect these problems, like valgrind on
Linux (one of the best) or the flag -fsanitize=address with clang. Then,
every object is allocated on its own page, making the program very
bloated and slow, but the program will then segfault immediately at the
point where the deleted object is accessed.
[*] not on the heap, so not "malloc/free-type freed", but "stack-freed"
Best regards,
Christian
Paavo Helde
May 19, 2021, 2:50:16 AM5/19/21
to
19.05.2021 06:06 Chris M. Thomasson kirjutas:
> On 5/18/2021 5:55 PM, Öö Tiib wrote:
>> The segfault indicates that gcc allows it to run as well unless you mean
>> that the compiler segfaults (that i've not seen gcc to do for ages).
>
> Yes, GCC allows it to run with warnings, and segfaults, and does not
> allow the bugged program to run to completion.
It's not GCC, but the hardware+OS which kills the running program. You
> On 5/18/2021 5:55 PM, Öö Tiib wrote:
>> The segfault indicates that gcc allows it to run as well unless you mean
>> that the compiler segfaults (that i've not seen gcc to do for ages).
>
> Yes, GCC allows it to run with warnings, and segfaults, and does not
> allow the bugged program to run to completion.
can compile the program with GCC for a platform without memory
protection (like 8086) and it will never segfault (because there are
neither segments nor faults). It might do other funny things though.
> MSVC allows that now. It compiles the bugged code, spits out a couple of
> warnings, and lets the compiled program run to completion. Strange.
OTOH, it's not normal for a programmer to ignore compiler warnings about
returning references to temporaries. You might want to use the "Treat
Specific Errors as Warnings" compiler feature for this warning
(/we"4172" with MSVC).
The only reason this is a warning and not an error is because UB
formally only appears when dereferencing the invalid pointer/reference,
returning it from a function is actually fine.
Chris M. Thomasson
May 19, 2021, 3:45:19 AM5/19/21
to
was allowed to run to completion under MSVC. Then I tried it GCC,
different things occurred. Its the nature of the beast wrt UB. I thank
you and everybody else for their time and patience.
Chris M. Thomasson
May 19, 2021, 3:46:52 AM5/19/21
to
again for the heads up on is_transparent!
James Kuyper
May 19, 2021, 9:59:00 AM5/19/21
to
On 5/19/21 1:57 AM, Christian Gollwitzer wrote:
...
correct. However, if it is, the correct way to describe your diagnosis
is to say that his code attempts to access the value stored in an object
after the lifetime of that object has ended. That description covers
both dynamically allocated objects that have been deallocated, and
automatic objects after the block in which they are defined exits.
...
> You are basically accessing memory that was "freed"[*] before. That is
...
> [*] not on the heap, so not "malloc/free-type freed", but "stack-freed"
I have not bothered checking his code to verify that your diagnosis is
correct. However, if it is, the correct way to describe your diagnosis
is to say that his code attempts to access the value stored in an object
after the lifetime of that object has ended. That description covers
both dynamically allocated objects that have been deallocated, and
automatic objects after the block in which they are defined exits.
Andrey Tarasevich
May 19, 2021, 8:44:34 PM5/19/21
to
On 5/18/2021 2:05 AM, Chris M. Thomasson wrote:
> I don't think this should work at all! It should segfault.
This statement makes no sense whatsoever. "Segfault" is typically a
> I don't think this should work at all! It should segfault.
specific manifestation of undefined behavior. There's no "should" about
undefined behavior.
--
Best regards,
Andrey Tarasevich
Chris M. Thomasson
May 19, 2021, 11:38:39 PM5/19/21
to
On 5/19/2021 5:44 PM, Andrey Tarasevich wrote:
> On 5/18/2021 2:05 AM, Chris M. Thomasson wrote:
>> I don't think this should work at all! It should segfault.
>
> This statement makes no sense whatsoever. "Segfault" is typically a
> specific manifestation of undefined behavior. There's no "should" about
> undefined behavior.
>
Well, yeah, you are right.
> On 5/18/2021 2:05 AM, Chris M. Thomasson wrote:
>> I don't think this should work at all! It should segfault.
>
> This statement makes no sense whatsoever. "Segfault" is typically a
> specific manifestation of undefined behavior. There's no "should" about
> undefined behavior.
>
0 new messages