[PATCH] D157948: [NFC]Fix possibly deref nullptr

[Wang, Xin via Phabricator via llvm-commits]

XinWang10 added a comment.

In D157948#4588502 <https://reviews.llvm.org/D157948#4588502>, @skan wrote:

> In D157948#4588131 <https://reviews.llvm.org/D157948#4588131>, @XinWang10 wrote:
>
>> In D157948#4587733 <https://reviews.llvm.org/D157948#4587733>, @skan wrote:
>>
>>> In D157948#4587327 <https://reviews.llvm.org/D157948#4587327>, @XinWang10 wrote:
>>>
>>>> In D157948#4587289 <https://reviews.llvm.org/D157948#4587289>, @skan wrote:
>>>>
>>>>> Is this a NFC change or a bug fix? If NFC, shouldn't we use assert?
>>>>
>>>> cast could use assert internally, I think it's same to explicit assert.
>>>
>>> Then how about others?
>>
>> OK, the other two places are in same function, line 432 shows
>>
>>   MMI = MMIWP ? &MMIWP->getMMI() : nullptr;
>>
>> So MMI could be nullptr, I just add add condition for those could deref this MMI, no func change.
>
> No, if `MMI` could be nullptr, then it's not a NFC change. If we never crash at this point, assert should be used instead.

Then I could remove NFC label.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:576
     case WinEH::EncodingType::Itanium:
-      ES = new WinException(this);
+      if (MMI)
+        ES = new WinException(this);
----------------
skan wrote:
> Why?
Constructor would deref MMI
```
WinException::WinException(AsmPrinter *A) : EHStreamer(A) {
  // MSVC's EH tables are always composed of 32-bit words.  All known 64-bit
  // platforms use an imagerel32 relocation to refer to symbols.
  useImageRel32 = (A->getDataLayout().getPointerSizeInBits() == 64);
...
const DataLayout &AsmPrinter::getDataLayout() const {
  return MMI->getModule()->getDataLayout();
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157948/new/

https://reviews.llvm.org/D157948