C ++ 멤버 함수에서 "if (! this)"는 얼마나 나쁩니 까?

C ++ 멤버 함수에서 "if (! this)"는 얼마나 나쁩니 까?

---

if (!this) return;앱에서 사용하는 오래된 코드를 발견 하면 위험이 얼마나 심각한가요? 즉각적인 앱 전체 검색 및 파괴 노력이 필요한 위험한 똑딱 거리는 시한 폭탄입니까, 아니면 조용히 제자리에 둘 수있는 코드 냄새와 비슷합니까?

물론이 작업을 수행하는 코드를 작성할 계획은 없습니다 . 오히려 최근에 우리 앱의 많은 부분에서 사용되는 오래된 핵심 라이브러리에서 무언가를 발견했습니다.

CLookupThingy클래스에 가상 이 아닌CThingy *CLookupThingy::Lookup( name ) 멤버 함수 가 있다고 상상해보십시오 . 카우보이 시절에 프로그래머 중 한 CLookupThingy *명이 함수 에서 NULL 이 전달되는 많은 충돌을 경험했으며 수백 개의 호출 사이트를 수정하는 대신 조용히 Lookup ()을 수정했습니다.

CThingy *CLookupThingy::Lookup( name ) 
{
   if (!this)
   {
      return NULL;
   }
   // else do the lookup code...
}

// now the above can be used like
CLookupThingy *GetLookup() 
{
  if (notReady()) return NULL;
  // else etc...
}

CThingy *pFoo = GetLookup()->Lookup( "foo" ); // will set pFoo to NULL without crashing

이번 주 초에이 보석을 발견했지만 지금은 고쳐야하는지에 대해 갈등이 생겼습니다. 이것은 우리의 모든 앱에서 사용하는 핵심 라이브러리에 있습니다. 이러한 앱 중 일부는 이미 수백만 고객에게 배송되었으며 제대로 작동하는 것 같습니다. 해당 코드에서 충돌이나 기타 버그가 없습니다. if !this조회 기능에서를 제거하면 잠재적으로 NULL을 전달하는 수천 개의 호출 사이트가 수정됩니다. 불가피하게 일부는 다음을 통해 무작위로 나타납니다 새로운 버그 도입, 놓칠 수없는 것이다 년 개발을.

그래서 나는 절대적으로 필요하지 않는 한 그것을 내버려 두는 경향이 있습니다.

기술적으로 정의되지 않은 동작이라는 점을 감안할 때 if (!this)실제로 얼마나 위험한 가요? 수리하는 데 수 주일의 노동 가치가 있습니까, 아니면 MSVC 및 GCC가 안전하게 돌아올 수 있다고 믿습니까?

우리의 앱은 MSVC 및 GCC에서 컴파일되며 Windows, Ubuntu 및 MacOS에서 실행됩니다. 다른 플랫폼으로의 이식성은 관련이 없습니다. 문제의 기능은 절대 가상이 아닙니다.

편집 : 내가 찾고있는 객관적인 대답의 종류는 다음과 같습니다.

• "MSVC 및 GCC의 현재 버전은 비가 상 멤버가 암시 적 'this'매개 변수가있는 정적 인 ABI를 사용하므로 'this'가 NULL 인 경우에도 안전하게 함수로 분기됩니다."또는
• "GCC의 향후 버전은 ABI를 변경하여 가상이 아닌 기능도 클래스 포인터에서 분기 대상을로드해야합니다."또는
• "현재 GCC 4.5는 때때로 비가 상 멤버를 암시 적 매개 변수가있는 직접 분기로 컴파일하고 때로는 클래스 오프셋 함수 포인터로 컴파일하는 일관성없는 ABI를 가지고 있습니다."

전자는 코드가 악취가 나지만 깨지지 않을 것임을 의미합니다. 두 번째는 컴파일러 업그레이드 후 테스트 할 것입니다. 후자는 높은 비용으로도 즉각적인 조치가 필요합니다.

분명히 이것은 발생하기를 기다리는 잠재적 버그이지만 지금은 특정 컴파일러의 위험을 완화하는 데만 관심이 있습니다.

---

나는 그것을 내버려 둘 것이다. 이것은 SafeNavigationOperator의 구식 버전으로 의도적으로 선택했을 수 있습니다 . 말했듯이 새 코드에서는 권장하지 않지만 기존 코드의 경우 그대로 둡니다. 당신이 그것을 수정한다면, 나는 그것에 대한 모든 호출이 테스트에 의해 잘 커버되도록 할 것입니다.

추가 편집 : 다음 을 통해 코드의 디버그 버전에서만 제거하도록 선택할 수 있습니다.

CThingy *CLookupThingy::Lookup( name ) 
{
#if !defined(DEBUG)
   if (!this)
   {
      return NULL;
   }
#endif
   // else do the lookup code...
}

따라서 프로덕션 코드에서 어떤 것도 손상되지 않으며 디버그 모드에서 테스트 할 수있는 기회를 제공합니다.

---

정의되지 않은 모든 동작처럼

if (!this)
{
   return NULL;
}

이것은 이다 꺼 기다리고 폭탄. 현재의 컴파일러에서 작동한다면 당신은 운이 좋고 운이 좋지 않습니다!

동일한 컴파일러의 다음 릴리스는 더 공격적 일 수 있으며이를 데드 코드로 간주합니다. 로가 thisnull이 될 수 없다, 코드는 "안전"제거 할 수 있습니다.

제거하면 더 좋다고 생각합니다!

---

많은 GetLookup 함수가 NULL을 반환하는 경우 NULL 포인터를 사용하여 메서드를 호출하는 코드를 수정하는 것이 좋습니다. 먼저

if (!this) return NULL;

와

if (!this) {
  // TODO(Crashworks): Replace this case with an assertion on July, 2012, once all callers are fixed.
  printf("Please mail the following stack trace to myemailaddress. Thanks!");
  print_stacktrace();
  return NULL;
}

이제 다른 작업을 계속하되 롤인 할 때 수정하십시오.

GetLookup(x)->Lookup(y)...

와

convert_to_proxy(GetLookup(x))->Lookup(y)...

conver_to_proxy는 NULL이 아닌 한 포인터를 변경하지 않고 반환합니다.이 경우 다른 답변에서와 같이 FailedLookupObject를 반환합니다.

---

비가 상 함수는 일반적으로 인라인되거나 "this"를 매개 변수로 사용하는 비 멤버 함수로 변환되기 때문에 대부분의 컴파일러에서 충돌하지 않을 수 있습니다. 그러나 표준은 특히 객체의 수명 밖에서 비 정적 멤버 함수를 호출하는 것은 정의되지 않았으며 객체의 수명은 객체에 대한 메모리가 할당되고 생성자가 완료되었을 때 시작되는 것으로 정의됩니다. 사소하지 않은 초기화.

이 표준은 생성 또는 소멸 중에 객체 자체가 호출하는 경우에만이 규칙에 예외를 적용하지만 가상 호출의 동작이 객체의 수명 동안 동작과 다를 수 있으므로주의해야합니다.

TL : DR : 모든 호출 사이트를 정리하는 데 오랜 시간이 걸리더라도 불로 죽일 것입니다.

---

향후 버전의 컴파일러는 공식적으로 정의되지 않은 동작의 경우보다 적극적으로 최적화 할 수 있습니다. 기존 배포 (컴파일러가 실제로 구현 한 동작을 알고있는 경우)에 대해서는 걱정하지 않지만 다른 컴파일러 나 다른 버전을 사용하는 경우 소스 코드에서 수정해야합니다.

---

이것은 '스마트하고 추한 해킹'이라고 불리는 것입니다. 참고 : 똑똑한! = 현명합니다.

리팩토링 도구없이 모든 호출 사이트를 찾는 것은 충분히 쉬울 것입니다. GetLookup ()을 어떻게 든 깨뜨려 컴파일되지 않도록 (예 : 서명 변경) 정적으로 오용을 식별 할 수 있습니다. 그런 다음이 모든 해킹이 지금 수행하는 작업을 수행하는 DoLookup ()이라는 함수를 추가합니다.

---

이 경우 멤버 함수에서 NULL 검사를 제거하고 비 멤버 함수를 만드는 것이 좋습니다.

CThingy* SafeLookup(CLookupThing *lookupThing) {
  if (lookupThing == NULL) {
    return NULL;
  } else {
    return lookupThing->Lookup();
  }
}

그러면 Lookup멤버 함수 에 대한 모든 호출을 쉽게 찾아 안전한 비 멤버 함수로 대체 할 수 있습니다.

---

If it's something that's bothering you today, it'll bother you a year from now. As you pointed out, changing it will almost certainly introduce some bugs -- but you can begin by retaining the return NULL functionality, add a bit of logging, let it run in the wild for a few weeks, and find how many times it even gets hit?

---

You can safely fix this today by returning a failed lookup object.

class CLookupThingy: public Interface {
  // ...
}

class CFailedLookupThingy: public Interface {
 public:
  CThingy* Lookup(string const& name) {
    return NULL;
  }
  operator bool() const { return false; }  // So that GetLookup() can be tested in a condition.
} failed_lookup;

Interface *GetLookup() {
  if (notReady())
    return &failed_lookup;
  // else etc...
}

This code still works:

CThingy *pFoo = GetLookup()->Lookup( "foo" ); // will set pFoo to NULL without crashing

---

This is a "ticking bomb" only if you are pedantic about the wording of the specification. However, regardless, it is a terrible, ill-advised approach because it obscures a program error. For that reason alone, I would remove it, even if it means considerable work. It is not an immediate (or even middle-term) risk, but it just isn't a good approach.

Such error hiding behavior really isn't something you want to rely on, either. Imagine you rely on this behavior (i.e. it doesn't matter whether my objects are valid, it will work anyway!) and then, by some hazard, the compiler optimizes out the if statement in a particular case because it can prove that this is not a null pointer. That is a legitimate optimization not just for some hypothetical future compiler, but for very real, present-time compilers as well.
But of course, since your program isn't well-formed, it happens that at some point you pass it a null this around 20 corners. Bang, you're dead.
That's very contrieved, admittedly, and it won't happen, but you cannot be 100% certain that it still cannot possibly happen.

Note that when I shout out "remove!", that does not mean the whole lot of them must be removed immediately or in one massive manpower operation. You could remove these checks one by one as you encounter them (when you change something in the same file anyway, avoid recompilations), or just text-search for one (preferrably in a highly used function), and remove that one.

Since you are using GCC, you may be intersted in __builtin_return_address, which may help you remove these checks without massive manpower and totally disrupting the whole workflow and rendering the application entirely unusable.
Before removing the check, modify it to to output the caller's address, and addr2line will tell you the location in your source. That way, you should be able to quickly identify all the locations in the application that are behaving wrongly, so you can fix these.

So instead of

if(!this) return 0;

change one location at a time to something like:

if(!this) { __builtin_printf("!!! %p\n", __builtin_return_address(0)); return 0; }

That lets you identify the invalid call sites for this change while still letting the program "work as intended" (you can also query the caller's caller if needed). Fix every ill-behaved location, one by one. The program will still "work" as normal.
Once no more addresses come up, remove the check alltogether. You might still have to fix one or the other crash if you are unlucky (because it didn't show while you tested), but that should be a very rare thing to happen. In any case, it should prevent your co-worker from shouting at you.
Remove one or two checks per week, and eventually none will be left. Meanwhile, life goes on and nobody notices what you're doing at all.

TL;DR
As for "current versions of GCC", you are fine for non-virtual functions, but of course nobody can tell what a future version might do. I deem it however highly unlikely that a future version will cause your code to break. Not few existing projects have this kind of check (I remember we had literally hundreds of them in Code::Blocks code completion at some time, don't ask me why!). Compiler makers probably don't want to make dozens/hundreds of major project maintainers unhappy on purpose, only to prove a point.
Also, consider the last paragraph ("from a logical point of view"). Even if this check will crash and burn with a future compiler, it will crash and burn anyway.

The if(!this) return; statement is somewhat useless insofar as this cannot ever be a null pointer in a well-formed program (it means you called a member function on a null pointer). This does not mean, of course, that it couldn't possibly happen. But in this case, the program should crash hard or abort with an assertion. Under no conditions should such a program continue silently.
On the other hand, it is perfectly possible to call a member function on an invalid object that happens to be not null. Checking whether this is the null pointer obviously doesn't catch that case, but it is the exact same UB. So, apart from hiding wrong behavior, this check also only detects one half of the problematic cases.

If you are going by the wording of the speficication, using this (which includes checking whether it's a null pointer) is undefined behavior. Insofar, strictly speaking, it is a "time bomb". However, I would not reasonably deem that a problem, both from a practical point of view and from a logical one.

• From a practical point of view, it doesn't really matter whether you read a pointer that is not valid as long as you do not dereference it. Yes, strictly to the letter, this is not allowed. Yes, in theory someone might build a CPU which will check invalid pointers when you load them, and fault. Alas, this isn't the case, if you're being real.
• From a logical point of view, assuming that the check will blow up, it still isn't going to happen. For this statement to be executed, the member function must be called, and (virtual or not, inlined or not) is using an invalid this, which it makes available inside the function body. If one illegitimate use of this blows up, the other will, too. Thus, the check is being obsoleted because the program already crashes earlier.

---

nb : 이 검사는 삭제 nullptr후 포인터를 설정하는 "안전 삭제 관용구"와 매우 유사 합니다 (매크로 또는 템플릿 safe_delete함수 사용). 아마도 이것은 동일한 포인터를 두 번 삭제하면 충돌하지 않기 때문에 "안전"합니다. 어떤 사람들은 중복 if(!ptr) delete ptr;.
아시다시피 operator delete는 널 포인터에서 작동하지 않는 것이 보장됩니다. 즉, 포인터를 널 포인터로 설정하는 것보다 더 많거나 적지 않게 이중 삭제를 감지 할 수있는 유일한 기회를 성공적으로 제거했습니다 (수정해야하는 프로그램 오류입니다!). "안전한"것은 아니지만 잘못된 프로그램 동작을 숨 깁니다. 개체를 두 번 삭제하면 프로그램 이 크게 중단됩니다.
삭제 된 포인터를 그대로 두거나 변조를 고집하는 경우 null이 아닌 유효하지 않은 포인터 (예 : 특수 "유효하지 않은"전역 변수의 주소 또는 다음과 같은 마법의 값)로 설정 -1해야합니다. 그러나 충돌이 발생할 때 속임수 를 쓰거나 숨기려고 해서는 안됩니다 .)

---

문제에 대해 경고하기 위해 가능한 한 빨리 실패해야한다는 것이 제 개인적인 의견입니다. 이 경우, 나는 내가 if(!this)찾을 수있는 모든 사건을 무의식적으로 제거 할 것이다.

참고 URL : https://stackoverflow.com/questions/8842886/how-bad-is-if-this-in-ac-member-function