[PATCH] D73890: Fix a -Wbitwise-conditional-parentheses warning in _LIBUNWIND_ARM_EHABI libunwind builds

[Nico Weber via Phabricator via llvm-commits]

thakis created this revision.
thakis added a reviewer: ajwong.
Herald added subscribers: christof, kristof.beyls.
thakis updated this revision to Diff 242055.
thakis added a comment.

Useful resources for reading the old ehabi code:

- Original code: https://reviews.llvm.org/rL211743
- EHABI spec: https://static.docs.arm.com/ihi0038/a/IHI0038A_ehabi.pdf?_ga=2.135617969.1216612656.1580737833-1865674396.1580737769


  src/UnwindCursor.hpp:1344:51: error: operator '?:' has lower precedence than '|';
      '|' will be evaluated first [-Werror,-Wbitwise-conditional-parentheses]
    _info.flags = isSingleWordEHT ? 1 : 0 | scope32 ? 0x2 : 0;  // Use enum?
                                        ~~~~~~~~~~~ ^
  src/UnwindCursor.hpp:1344:51: note: place parentheses around the '|' expression
      to silence this warning
    _info.flags = isSingleWordEHT ? 1 : 0 | scope32 ? 0x2 : 0;  // Use enum?
                                                    ^
                                        (          )
  src/UnwindCursor.hpp:1344:51: note: place parentheses around the '?:' expression
      to evaluate it first
    _info.flags = isSingleWordEHT ? 1 : 0 | scope32 ? 0x2 : 0;  // Use enum?
                                                    ^
                                            (                )

But `0 |` is a no-op for either of those two interpretations, so I think what was meant here was

  _info.flags = (isSingleWordEHT ? 1 : 0) | (scope32 ? 0x2 : 0);  // Use enum?

Previously, if `isSingleWordEHT` was set, bit 2 would never be set. Now it is. From what I can tell, the only thing that checks these bitmask is ProcessDescriptors in Unwind-EHABI.cpp, and that only cares about bit 1, so in practice this shouldn't have much of an effect.


https://reviews.llvm.org/D73890

Files:
  libunwind/src/UnwindCursor.hpp


Index: libunwind/src/UnwindCursor.hpp
===================================================================
--- libunwind/src/UnwindCursor.hpp
+++ libunwind/src/UnwindCursor.hpp
@@ -1353,7 +1353,8 @@
 
   // If the high bit is set, the exception handling table entry is inline inside
   // the index table entry on the second word (aka |indexDataAddr|). Otherwise,
-  // the table points at an offset in the exception handling table (section 5 EHABI).
+  // the table points at an offset in the exception handling table (section 5
+  // EHABI).
   pint_t exceptionTableAddr;
   uint32_t exceptionTableData;
   bool isSingleWordEHT;
@@ -1452,7 +1453,7 @@
   _info.unwind_info = exceptionTableAddr;
   _info.lsda = lsda;
   // flags is pr_cache.additional. See EHABI #7.2 for definition of bit 0.
-  _info.flags = isSingleWordEHT ? 1 : 0 | scope32 ? 0x2 : 0;  // Use enum?
+  _info.flags = (isSingleWordEHT ? 1 : 0) | (scope32 ? 0x2 : 0);  // Use enum?
 
   return true;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73890.242055.patch
Type: text/x-patch
Size: 970 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200203/8bb46d93/attachment.bin>