[PATCH] D101353: [DebugInfo] do not add pc value if lsda value in fde is 0

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

A test plan of `check-llvm` is not sufficient.  It shows that you didn't break anything that we have an existing test for, but it would also suggest that there is no test for this case, and you want that so someone else doesn't break your change in the future.



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:1140
+
+            auto IsPCRelativeFn = [&]() {
+              return (0x70 & Cie->getLSDAPointerEncoding()) ==
----------------
For a bool computation that is used only once and not in a loop, a local variable rather than a lambda seems simpler and more efficient.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:1142
+              return (0x70 & Cie->getLSDAPointerEncoding()) ==
+                     dwarf::DW_EH_PE_pcrel;
+            };
----------------
It seems that `DW_EH_PE_aligned` is overloading the `DW_EH_PE_pcrel` flag, and that's why you're doing the test this way?  I see `DWARFDataExtractor::getEncodedPointer` is doing something similar.
Please add an enum constant to Dwarf.h for the 0x70 mask, and use that.


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

https://reviews.llvm.org/D101353