How to prevent returning a pointer to a temporary variable?

Question

On a recent bug hunt, I found an issue with returning a pointer to a member of a temporary variable. The offending (simplified) code was:

struct S {
    S(int i) : i(i) {}
    int i;
    int* ptr() { return &i; }
};

int* fun(int i) { return S(i).ptr(); }  // temporary S dies but pointer lives on

int main() {
    int* p = fun(1);
    return *p;  // undefined
}

How to prevent this? GCC & Clang have -Waddress-of-temporary and -Wreturn-stack-address but they seem to loose trail because of ptr() acting as a middle man for the dirty deeds. They are only triggered when the pointer is taken directly:

int* fun(int i) { return &S(i).i; }  // rightly fails to compile

My project also incorporates cppcheck in continuous integration but it also can't pick it up (raised here).

Which static analysis tool can prevent this class of bugs?

EDIT: GCC does pick it up since version 6.1.0 with -Wreturn-local-addr and (surprisingly) -O2 switched on.

Vibrate

If you compile with -O2, gcc catches this bug: melpon.org/wandbox/permlink/KaXY4ktTM1vMNTiX – Salta

"How to prevent this?" there is only one solution - stop using raw pointers – Portaltoportal

@Salta -O2 has nothing to do with it but the version of GCC does (detects since 6.1.0). I can't use a GCC that new, but it's a good argument to troll our maintainers to switch to a newer one:) – Ghassan

@TomekSowiński It doesn't show this warning when you compile without -O2 on wandbox. But It's still a good idea to update :) – Salta

Well, int* ptr() & { return &i; } works in your specific case, but you can easily get the issue back by doing int* fun(int i) { S s{i}; return s.ptr(); }. – Lailalain

@Salta ah, you're right (it doesn't detect with or without -O2 before 6.1.0, though). Come to think again... why would an optimization switch influence static analysis? I may file a bug for GCC:) – Ghassan

@Portaltoportal not really, same happens with references. Besides, there are valid reasons to return a member by reference (pointer). – Ghassan

@Portaltoportal What he really means is "How to get the compiler to warn me so I'll know to fix it?" Obviously the compiler can't prevent you from doing it in the first place. – Gripping

@TomekSowiński yes same happens, bad design is a bad design either by pointer or reference – Portaltoportal

To maintainers: I'm asking which tool can detect this programming error, which is not a matter of opinion and doesn't attract opinionanted answers. Please don't close this question. – Ghassan

@Gripping obviously compiler can prevent you only in trivial cases. Create a local variable and call that method and it is not possible to detect that – Portaltoportal

"why would an optimization switch influence static analysis?" Because GCC is not a static analyzer. – Mcgean

I have created a ticket on Cppcheck's bug tracker about this issue in order to catch issues like this in the future. Ticket: trac.cppcheck.net/ticket/7812#ticket – Demivolt

@TomekSowiński asking for third party tools is considered off-topic here – Rader

Slightly off-topic, but avoiding these classes of bugs is the raison d'être of Rust. Learning Rust made me a better C++ developer. – Pistoia

You should not write code like that in 2016... First, you should avoid as much as possible to return pointer/reference to internal objects (bad design) and if you need, then you should use smart pointers instead. – Limekiln

The problem with the code above is that the compiler need to do multiple level analysis to detect the problem. It it legal to return the address of a member of a class as long as you don't use that pointer after the containing object is destroyed. And then in your fun function, the compiler won't check the function and assume that it return a pointer that do not depend on the object. With optimization, the compiler would do inlining and see it as it would be a single function. With external function, it would not be able to do the check (at least without global analysis). – Limekiln

Answer 1

I am a Cppcheck developer.

My project also incorporates cppcheck in continuous integration but it also can't pick it up.

interesting bug. This is the kind of bug that cppcheck wants to warn about. We have some related checks but this slipped through unfortunately.

Not really surprising given the regex nature of cppcheck.

I don't personally understand why some people say cppcheck is a regex tool.

It uses AST, context sensitive valueflow analysis, etc to detect bugs. So does GCC and Clang. Cppcheck is sometimes claimed to be a regex tool but GCC and Clang are not.

Bobbinet

FYI: A ticket about this issue is already created trac.cppcheck.net/ticket/7812 – Demivolt

I admit I didn't look into cppcheck internals and probably got the impression it's regex-based on the way custom rules are defined. I removed that remark from the question not to proliferate wrong info. – Ghassan